Static code analysis warnings

[ben-sedg]

Hi Matias, I got warnings from my static code analysis. I am fixing them on my side. I was wondering if you can also fix them. (If i have access to push branches i can also do it) Here are the warnings I get. The line number might be off by 1 or 2.

MdnsLib.h:1140:29: warning: Either the condition '!data' is redundant or there is overflow in pointer subtraction. [nullPointerArithmeticRedundantCheck] size_t remain = capacity - MDNS_POINTER_DIFF(data, buffer); ^ MdnsLib.h:1141:6: note: Assuming that condition '!data' is not redundant if (!data || (remain <= 4)) ^ MdnsLib.h:1140:29: note: Null pointer subtraction size_t remain = capacity - MDNS_POINTER_DIFF(data, buffer); ^ MdnsLib.h:1155:29: warning: Either the condition '!data' is redundant or there is overflow in pointer subtraction. [nullPointerArithmeticRedundantCheck] size_t remain = capacity - MDNS_POINTER_DIFF(data, buffer); ^ MdnsLib.h:1156:6: note: Assuming that condition '!data' is not redundant if (!data || (remain < 10)) ^ MdnsLib.h:1155:29: note: Null pointer subtraction size_t remain = capacity - MDNS_POINTER_DIFF(data, buffer); ^ MdnsLib.h:1248:23: warning: Either the condition '!data' is redundant or there is overflow in pointer subtraction. [nullPointerArithmeticRedundantCheck] remain = capacity - MDNS_POINTER_DIFF(data, buffer); ^ MdnsLib.h:1249:7: note: Assuming that condition '!data' is not redundant if (!data || (remain <= string_length) || (string_length > 0x3FFF)) ^ MdnsLib.h:1248:23: note: Null pointer subtraction remain = capacity - MDNS_POINTER_DIFF(data, buffer); ^ MdnsLib.h:709:9: style: The scope of the variable 'pos' can be reduced. [variableScope] size_t pos = 0; ^ MdnsLib.h:1461:9: style: The scope of the variable 'separator' can be reduced. [variableScope] size_t separator, sublength; ^ MdnsLib.h:1461:20: style: The scope of the variable 'sublength' can be reduced. [variableScope] size_t separator, sublength; ^ MdnsLib.h:709:13: style: Variable 'pos' is assigned a value that is never used. [unreadVariable] size_t pos = 0;

[mjansson]

Thanks, I will have a look at it

[mjansson]

I don't really agree with the analysis though, the null pointer check and remain subtraction are neither redundant nor unsafe, it's just condensed into a single if-return statement instead of split up like

data = ...; if (!data) return data; remain = ...; if (remain <= 4) return data;

Makes the code more compact and easier to read. It does not matter that the subtraction will overflow if data is null as the combined conditional will catch it anyway.

I fixed the variable scopes, those made perfect sense.

[hmh]

It is complaining that you should not do pointer arithmetric on NULL, actually. Which is correct (if pedantic): the C standard does not allow sums or subtractions with NULL.

And an annoyingly bug-introducing optimizing compiler can quite nicely decide that !data is just plain impossible because you did a sum/subtraction using data right before, and skip the check.

However, the "uncompressed" code you posted in the comment checks for !data and returns before it tries to calculate "remain", and that would not cause issues.

[mjansson]

The spectre of undefined behaviour :D Fair enough, although a compiler doing that would probably break half the world in the process.

[hmh]

yeah, but when dealing with this kind of issue, it is always best to use the "less compact" version that checks for NULL before anything that might either do math on the pointer, or derreference it... it might just save someone from completely surprising, dangerous behavior that might be quite a pain to track down, and won't be easy to reproduce if you're using a different compiler, etc.

The static analyzer error message is quite hard to understand, though. Agreed on that...

[mjansson]

Fixed