laurencelundblade / QCBOR

Comprehensive, powerful, commercial-quality CBOR encoder/ decoder that is still suited for small devices.
Other
183 stars 47 forks source link

Fix warning from static analysis #178

Closed escherstair closed 1 year ago

escherstair commented 1 year ago

const-qualify *half to silence the warning

laurencelundblade commented 1 year ago

Hi

The change to %u makes sense.

The addition of const seems OK, but the prototype in half_to_double_from_rfc7049.h has to have the argument const.

Can you say a bit more about error you got from the static analyzer without the const? Which static analyzer? I'm a bit surprised that there is a problem here when I look where it is called from -- the arguments passed aren't const.

I'm a big fan of const, so the change seems good, but would like to understand the error and the header has to be fixed.

Thank you for taking the time to file the PR!

LL

escherstair commented 1 year ago

The addition of const seems OK, but the prototype in half_to_double_from_rfc7049.h has to have the argument const.

Done

Can you say a bit more about error you got from the static analyzer without the const? Which static analyzer? I'm a bit surprised that there is a problem here when I look where it is called from -- the arguments passed aren't const.

I use cppcheck as a static analyzer tool. It warns about several kind of potential issues, from errrors to style suggestions. The const is not an error, but a warning. The reason is that, when a function takes an input parameter as pointer, but it doesn't change the pointed, the code is more expressive if the parameter is declared as const* In this way the user of the function (maybe it's not the guy that wrote the function) sees clearly that the function won't modify the pointed variable (without having to look to the implementation code). But the user can pass a variable that in the caller contest is not const, as in the the following snippet:

char string[50];
snprintf(string, sizeof(string), "this string is not const");
double d = decode_half(string); /* it's clear that string won't be modified */
laurencelundblade commented 1 year ago

OK. Makes sense what you said about cppcheck. Agree with the reasons for const her :-)

Should get to merging in the next day or so.

Thanks!