|
From: Ivo R. <ivo...@gm...> - 2015-07-31 23:13:16
|
I have come recently to very strange piece of code in function
VG_(load_ELF)(), when it iterates over program headers.
I think that all switch cases after PT_INTERP are misplaced.
That is, the closing bracket for case PT_INTERP is located
*after* the last "default" case.
So what happens with cases PT_GNU_STACK, PT_SUNWSTACK
and default is a mystery for me.
I dug into file history across renames
(coregrind/ume.c -> coregrind/m_ume.c -> coregrind/m_ume/elf.c)
and found that the default case was possibly added at a wrong
position at r2659. At r2643 everything looked ok.
r2643:
switch(ph->p_type) {
...
case PT_INTERP: {
...
for(j = 0; j < interp->e.e_phnum; j++) {
...
}
break;
} // for case PT_INTERP
} // for switch
r2659:
switch(ph->p_type) {
...
case PT_INTERP: {
...
for(j = 0; j < interp->e.e_phnum; j++) {
...
}
break;
default: <---- misplaced
// do nothing
break;
} // for case PT_INTERP
} // for switch
Do you agree with my findings?
What I don't understand is why compiler does not flag this as an error?
I.
|
|
From: John R. <jr...@bi...> - 2015-08-01 06:01:35
|
> r2659:
> switch(ph->p_type) {
> ...
> case PT_INTERP: {
> ...
> for(j = 0; j < interp->e.e_phnum; j++) {
> ...
> }
> break;
>
> default: <---- misplaced
> // do nothing
> break;
> } // for case PT_INTERP
> } // for switch
>
> Do you agree with my findings?
Yes. Almost certainly the 'default' should not be inside the "{ ... }"
for "case PT_INTERP:".
For "safety belt" style, I like:
default: { // occurs FIRST and ALWAYS
...
} break;
case K: {
...
} break;
where EVERY 'case' and 'default' has braces and a following 'break'.
For fall-through:
case K: {
...
} // FALL THROUGH
> What I don't understand is why compiler does not flag this as an error?
It is legal C. https://en.wikipedia.org/wiki/Duff%27s_device#Mechanism
|
|
From: Ivo R. <ivo...@gm...> - 2015-08-01 09:00:32
|
2015-08-01 8:01 GMT+02:00 John Reiser <jr...@bi...>: > > > What I don't understand is why compiler does not flag this as an error? > > It is legal C. https://en.wikipedia.org/wiki/Duff%27s_device#Mechanism > > Thanks for the link! It made things clear for me. I. |