On Thu, Aug 31, 2017 at 05:10:27PM +0200, Boudewijn Dijkstra wrote:
> Op Thu, 31 Aug 2017 14:08:07 +0200 schreef Otto Moerbeek <o...@drijf.net>:
> > /usr/src/usr.sbin/sasyncd/carp.c:157:12: warning: comparison of
> > unsigned enum expression < 0 is always false [-Wtautological-compare]
> > if (state < 0 || state > FAIL)
> > ~~~~~ ^ ~
> > /usr/src/usr.sbin/sasyncd/carp.c:166:20: warning: comparison of
> > unsigned enum expression < 0 is always false [-Wtautological-compare]
> > if (current_state < 0 || current_state > FAIL) {
> > ~~~~~~~~~~~~~ ^ ~
> >
> > this warning is a tiny bit interesting. A compiler is free to choose
> > the type of the enum, as long as it can represent all given values.
> > So another compiler might choose not to make it unsigned. So I came up
> > with this fix that is not depending on the signedness of the type. But
> > most of the time avoiding enum is better, I suppose.
> >
> > -Otto
> >
> > Index: carp.c
> > ===================================================================
> > RCS file: /cvs/src/usr.sbin/sasyncd/carp.c,v
> > retrieving revision 1.15
> > diff -u -p -r1.15 carp.c
> > --- carp.c 27 Aug 2016 04:21:08 -0000 1.15
> > +++ carp.c 31 Aug 2017 12:03:59 -0000
> > @@ -154,7 +154,7 @@ carp_state_name(enum RUNSTATE state)
> > {
> > static const char *carpstate[] = CARPSTATES;
> > - if (state < 0 || state > FAIL)
> > + if ((unsigned)state > FAIL)
> > state = FAIL;
> > return carpstate[state];
> > }
> > @@ -163,7 +163,7 @@ void
> > carp_update_state(enum RUNSTATE current_state)
> > {
> > - if (current_state < 0 || current_state > FAIL) {
> > + if ((unsigned)current_state > FAIL) {
> > log_err("carp_update_state: invalid carp state, abort");
> > cfgstate.runstate = FAIL;
> > return;
> >
>
> Doesn't the compiler warn about enum non-member assignment? If it does, then
> checking it would be redundant here, no?
Only in C++, which has stricter rule regarding to enums.
-Otto
