InterLinked1
commented
1 year ago The usage of sizeof looks fine to me. You could change the first one, line 209 to
sizeof(struct mdcparams), but it would do the same thing it's doing now. The only reason to change it would be make it look more like the allocation on line 204.
These both look similar to the last issue Danny identified, where it's taking the address of a pointer. That is almost certainly not what is intended here. I think anything in the codebase with sizeof(& is probably highly suspect, and likely wrong.
The typical usage for this type of thing with a pointer is to use *, to dereference, rather than &, which does the opposite, and gets the address of the variable. Taking the sizeof of a pointer in general will just give the size of a pointer (4 or 8), which is also wrong. Given the variable is a pointer, this gets the address of the pointer. Now add sizeof and you have the size of a pointer on your system. 100% not what we want.
In contrast, using * instead is very common and correct.
As discussed before, sizeof should not be used directly for strlen. The usage is wrong and confusing. If the string is null terminated, I don't see why strcmp could not be used.
encbar5
The following functions appear to use the sizeof() function incorrectly.
apps/app_rpt/rpt_mdc1200.c In mdc_1200_ack_status, line 209:
memset(mdcp, 0, sizeof(&mdcp));This should be sizeof(mdcparams)
apps/app_rpt/rpt_uchameleon.c In uchameleon_connect, line 73:
if ((count != 13) || (strncmp(expect, rxbuf + 4, sizeof(&expect)))) {expect may need to be changed to a character array or change sizeof(&expect), or the literal 9, or strlen(expect).
Note: I am not sure why idbuf, ledbuf, and expect are declared as static. They probably should be char arrays and const.