[Dnsmasq-discuss] Three v2.92test8 findings (one regression from d1008215 made 2025 May 14)

[Simon Kelley]

On 5/16/25 17:46, Matthias Andree wrote:
> Hi Simon,
> 
> compiling dnsmasq v2.92test8 on FreeBSD 14.2-RELEASE (which uses clang 
> 18 as default compiler) yields a new warning:
> 
>> tftp.c:365:7: warning: variable 'filename' is used uninitialized 
>> whenever '||' condition is true [-Wsometimes-uninitialized]
>>  365 |   if (ntohs(*((unsigned short *)packet)) != OP_RRQ ||
>>      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> /usr/include/netinet/in.h:121:18: note: expanded from macro 'ntohs'
>>  121 | #define ntohs(x)        __ntohs(x)
>>      |                         ^
>> /usr/include/sys/_endian.h:89:20: note: expanded from macro '__ntohs'
>>   89 | #define __ntohs(x)      (__bswap16(x))
>>      |                         ^
>> tftp.c:370:12: note: uninitialized use occurs here
>>  370 |       if (!filename)
>>      |            ^~~~~~~~
>> tftp.c:365:7: note: remove the '||' if its condition is always false
>>  365 |   if (ntohs(*((unsigned short *)packet)) != OP_RRQ ||
>>      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> /usr/include/netinet/in.h:121:18: note: expanded from macro 'ntohs'
>>  121 | #define ntohs(x)        __ntohs(x)
>>      |                         ^
>> /usr/include/sys/_endian.h:89:20: note: expanded from macro '__ntohs'
>>   89 | #define __ntohs(x)      (__bswap16(x))
>>      |                         ^
>> tftp.c:48:17: note: initialize the variable 'filename' to silence this 
>> warning
>>   48 |   char *filename, *mode, *p, *end, *opt;
>>      |                 ^
>>      |                  = NULL
>>
> This is with our default compiler on FreeBSD, clang 18. (I tried GCC 15, 
> which does not warn about this.)
> 
>> $ cc --version
>> FreeBSD clang version 18.1.6 (https://github.com/llvm/llvm-project.git 
>> llvmorg-18.1.6-0-g1118c2e05e67)
>> Target: x86_64-unknown-freebsd14.2
>> Thread model: posix
>> InstalledDir: /usr/bin
>>
> And was apparently introduced this week with this commit:
> 
>> commit d1008215dc95334e88a4a8e9d0ba0975a94c1f63
>> Author: Simon Kelley <simon at thekelleys.org.uk>
>> Date:   Wed May 14 21:15:17 2025 +0100
>>
>>    Better error message when rejecting a TFTP transfer.
>>
> I don't oversee enough of the code to judge whether the first-16bits-of- 
> packet-in-network-order != OP_RRQ is relevant here. If it's from the 
> network, of course any garbage can be in there and kick the reporting in 
> lines 370ff off balance.
> 
> What do you think?

Clang is right, gcc missed a trick. re-ordering the tests make it 
sensible, as far as I can see. Fixed in

https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=baf3c57af5d432a8239f7c28c414aa41d95a1bb6
> 
> 
> Also, lower priority,
> 
> finding #2, the attached patch fixes a warning about an unused static 
> function:
> 
> forward.c:673:23:warning: 'domain_find_sets' defined but not used [- 
> Wunused-function]
>   673 | static struct ipsets *domain_find_sets(struct ipsets *setlist, 
> const char *domain) {
>       | ^~~~~~~~~~~~~~~~
> 
> 
Not a problem, but certainly noise. Also fixed in the same commit. There 
are a few more of these around, that generate spurious unused 
variable/function warnings when certain compile-time settings are in 
effect. It would be nice to fix them so there's no warnings when I run 
my "compile dnsmasq with every sane combination of compile-time-options" 
script. To reach that state, I'd also have to fix the ubus header files :-(



> finding #3: GCC15 is a bit overzealous with its warnings and displays 
> this on what seems reasonable code to me:
> 
>> edns0.c:In function 'add_umbrella_opt':
>> edns0.c:512:30:warning: initializer-string for array of 'unsigned 
>> char' truncates NUL terminator but destination lacks 'nonstring' 
>> attribute (5 chars into 4 available) [-Wunterminated-string- 
>> initialization]
>>  512 |   struct umbrella_opt opt = {{"ODNS"}, UMBRELLA_VERSION, 0, {0}};
>>      | ^
> 

It seems reasonable code to me too, and a any fix would be much more 
ugly. Ignoring.

Cheers,

Simon.


> Cheers,
> Matthias
>