Message ID | 872cdd55-2333-4304-9335-3ca5574f30c5@BAMAIL02.ba.imgtec.org |
---|---|
State | New |
Headers | show |
On 12/10/2014 07:19 PM, Steve Ellcey wrote: > Here is another warning/error fix. The mips dl-trampline.c is doing a > switch on a comparision expression and this gives the error: > > ../sysdeps/mips/dl-trampoline.c: In function '__dl_runtime_resolve': > ../sysdeps/mips/dl-trampoline.c:142:15: error: switch condition has boolean value [-Werror=switch-bool] > switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > ^ > cc1: all warnings being treated as errors > > I looked at changing the switch to an if statement because there are only > two cases (default and 0) but it turned out to be a bit tricky because > in some cases the default case does a break and in other cases it falls > through to the '0' case. So instead I just cast the comparision to an > int before switching on it. > > Ok to checkin? Why not "switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL ? 0 : 1)" to avoid the (int) case potentially hiding future type errors? > Steve Ellcey > sellcey@imgtec.com > > > 2014-12-10 Steve Ellcey <sellcey@imgtec.com> > > * sysdeps/mips/dl-trampoline.c: Cast switch expression. > > diff --git a/sysdeps/mips/dl-trampoline.c b/sysdeps/mips/dl-trampoline.c > index f565654..3aa147b 100644 > --- a/sysdeps/mips/dl-trampoline.c > +++ b/sysdeps/mips/dl-trampoline.c > @@ -139,7 +139,7 @@ __dl_runtime_resolve (ElfW(Word) sym_index, > /* FIXME: The symbol versioning stuff is not tested yet. */ > if (__builtin_expect (ELFW(ST_VISIBILITY) (sym->st_other), 0) == 0) > { > - switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > + switch ((int) (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL)) > { > default: > { >
On Wed, 2014-12-10 at 20:01 -0500, Carlos O'Donell wrote: > Why not "switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL ? 0 : 1)" to > avoid the (int) case potentially hiding future type errors? That seems reasonable but shouldn't it be "switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL ? 1 : 0)" I.e. return 1 if the boolean expression is true and 0 if the boolean expression is false? Steve
On 12/11/2014 11:21 AM, Steve Ellcey wrote: > On Wed, 2014-12-10 at 20:01 -0500, Carlos O'Donell wrote: > >> Why not "switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL ? 0 : 1)" to >> avoid the (int) case potentially hiding future type errors? > > That seems reasonable but shouldn't it be "switch (l->l_info[VERSYMIDX > (DT_VERSYM)] != NULL ? 1 : 0)" I.e. return 1 if the boolean expression > is true and 0 if the boolean expression is false? Yes, you are correct, my mistake. Cheers, Carlos.
diff --git a/sysdeps/mips/dl-trampoline.c b/sysdeps/mips/dl-trampoline.c index f565654..3aa147b 100644 --- a/sysdeps/mips/dl-trampoline.c +++ b/sysdeps/mips/dl-trampoline.c @@ -139,7 +139,7 @@ __dl_runtime_resolve (ElfW(Word) sym_index, /* FIXME: The symbol versioning stuff is not tested yet. */ if (__builtin_expect (ELFW(ST_VISIBILITY) (sym->st_other), 0) == 0) { - switch (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL) + switch ((int) (l->l_info[VERSYMIDX (DT_VERSYM)] != NULL)) { default: {