Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use of sizeof() operator #20

Closed
MaJerle opened this issue Aug 10, 2018 · 1 comment
Closed

Use of sizeof() operator #20

MaJerle opened this issue Aug 10, 2018 · 1 comment
Labels

Comments

@MaJerle
Copy link

MaJerle commented Aug 10, 2018

Repeating yourself is generally bad and can easily lead to bugs when updating software. I suggest to change code in functions like _ntoa_long_long:

char buf[PRINTF_NTOA_BUFFER_SIZE];
....
do { 
...
} while (len < PRINTF_NTOA_BUFFER_SIZE);

to:

char buf[PRINTF_NTOA_BUFFER_SIZE];
....
do { 
...
} while (len < (sizeof(buf) / sizeof(buf[0])));

or if typing twice sizeof is too much, you may do:

#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

and then

} while (len < ARRAY_SIZE(buf));

Specially important if this is automotive ready code.

@mpaland
Copy link
Owner

mpaland commented Aug 10, 2018

Hello @MaJerle,
thanks a lot for your suggestion about the sizeof() operator! Appreciate your feedback very much.

PRINTF_NTOA_BUFFER_SIZE is used as a (macro defined) module scoped constant here.
I would agree to use ARRAY_SIZE or COUNTOF macros if the buffer size is set by a magic number, is externally defined or depends on element initialization (buffer[] = { ... };) etc.
But in this case here using a well defined constant (even named XXX_BUFFER_SIZE) for setting the buffer size and checking its length in different expressions is perfectly fine (and automotive safe).
Using 'ARRAY_SIZE' has the advantage that one can define a buffer with an arbitrary size and the bounds check always works, that's right - but IMHO not mandatory necessary here.

I don't see., why and how this should lead to a bug here, except one would change char buf[PRINTF_NTOA_BUFFER_SIZE] to something like char buf[20] or char buf[PRINTF_NTOA_BUFFER_SIZE - 10U] - but that would be a serious violation (which coverity should find anyway).

Besides PRINTF_NTOA_BUFFER_SIZE is used in _ntoa_format() where buf is unknown as buffer (it's just a pointer there).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants