DIGImend / hidrd

HID report descriptor I/O library and conversion tool
GNU General Public License v2.0
171 stars 29 forks source link

Verify comparisons between enum variables and hardcoded constants #6

Closed ao2 closed 10 years ago

ao2 commented 10 years ago

Hi,

I was playing with warning flags and other compilers, and clang -Wall gives some warnings:

../include/hidrd/item/pfx/size.h:60:35: warning: comparison of constant 4 with expression of type 'hidrd_item_pfx_size' (aka 'enum hidrd_item_pfx_size') is
      always false [-Wtautological-constant-out-of-range-compare]
    return (bytes <= 2) || (bytes == 4);

In enum hidrd_item_pfx_size you never set a value to 4.

There are other similar warnings.

spbnick commented 10 years ago

Thank you, Antonio! I fixed a bunch of other Clang warnings as well.

ao2 commented 10 years ago

Great.

About the other warnings: casting in printf can often be avoided by picking the right format argument, but I don't know the specifics here, so I trust you.

BTW, what would be the best way to add stricter compilation flags to hidrd? (-std=c99 -pedantic, etc.)? I see that you reset CFLAGS in an unusual way in configure.ac, IIRC Makefile.am is the standard location for CFLAGS (re)definition.

spbnick commented 10 years ago

Thanks for the printf hint. In this case (0beaf93138c19f8a17300992a943a85c12b9f8c5) Clang was complaining about (promoted?) constant arguments which were actually supposed to be of specific types to which the commit casts them, so in my understanding this is correct.

The particular way CFLAGS is handled was a rather mindless carry-over from projects at work at the time. I should probably rethink it, read some docs and do it in a less unusual and a more useful way. Thanks for the comment. Meanwhile you'll probably need to repeat the default CFLAGS on the make command line if you want to add them.