Fails to compile with gcc 10.2.1: error: comparison is always true due to limited range of data type

[bombadil]

When trying to package smcroute 2.5.2 for Debian unstable, it failed to compile

build@host:/tmp/build/smcroute-2.5.2$ make
make  all-recursive
make[1]: Verzeichnis „/tmp/build/smcroute-2.5.2“ wird betreten
Making all in man
make[2]: Verzeichnis „/tmp/build/smcroute-2.5.2/man“ wird betreten
make[2]: Für das Ziel „all“ ist nichts zu tun.
make[2]: Verzeichnis „/tmp/build/smcroute-2.5.2/man“ wird verlassen
Making all in src
make[2]: Verzeichnis „/tmp/build/smcroute-2.5.2/src“ wird betreten
gcc -DHAVE_CONFIG_H -I. -I..  -D_ATFILE_SOURCE -D_INCOMPLETE_XOPEN_C063 -DSYSCONFDIR=\"/etc\" -DRUNSTATEDIR=\"/run\" -Wdate-time -D_FORTIFY_SOURCE=2 -W -Wall -Wextra -Wno-deprecated-declarations -std=gnu99  -g -O2 -ffile-prefix-map=/tmp/build/smcroute-2.5.2=. -fstack-protector-strong -Wformat -Werror=format-security -ggdb -O0 -Werror -ggdb -c -o smcrouted-mroute.o `test -f 'mroute.c' || echo './'`mroute.c
mroute.c: In function ‘mroute4_del_vif’:
mroute.c:312:17: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  312 |  if (iface->vif >= 0 && iface->vif < ALL_VIFS)
      |                 ^~
mroute.c: In function ‘mroute6_del_mif’:
mroute.c:914:17: error: comparison is always true due to limited range of data type [-Werror=type-limits]
  914 |  if (iface->mif >= 0 && iface->mif < ALL_VIFS)
      |                 ^~
cc1: all warnings being treated as errors
make[2]: *** [Makefile:525: smcrouted-mroute.o] Fehler 1
make[2]: Verzeichnis „/tmp/build/smcroute-2.5.2/src“ wird verlassen
make[1]: *** [Makefile:490: all-recursive] Fehler 1
make[1]: Verzeichnis „/tmp/build/smcroute-2.5.2“ wird verlassen
make: *** [Makefile:352: all] Fehler 2
build@host:/tmp/build/smcroute-2.5.2$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

build@host:/tmp/build/smcroute-2.5.2$ 

[troglobit]

Ah, I see you're building with -Werror, I noticed this warning just prior to the release. I guess, since vifi_t is unsigned char, the comparison should be if (iface->vif != ALL_VIFS).

[bombadil]

Hmm, or just drop the first condition of the IF clause to maintain the original intent?

if (iface->vif < ALL_VIFS)

[troglobit]

What original intent? I rewrote that whole file this summer :-)

[bombadil]

I just meant using the smaller than operator matches more accurately what values of vif are valid than the not equal operator. So, the original intent certainly was to be as accurate as possible. ;)

[troglobit]

Fixed, you can backport it if you like. I don't plan to do another patch release in a while.

Also, heads-up: I've added a lot of new features, e.g. native /etc/smcroute.d support. Iirc there's something in Debian today that has a local implementation of the same?

[bombadil]

Ok, thanks for the lightning fast fix and the hint. I'll take a look sooner or later.