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

Fix array underflow compiler warnings #226

Merged
merged 3 commits into from Sep 27, 2016
Merged

Conversation

smcv
Copy link

@smcv smcv commented Sep 25, 2016

gcc 6 warns about some array underflows while compiling ioquake3. Please see the individual commit messages for more details.

Given an array b[] of length n, pointers to &b[0]..&b[n] are defined
(where only &b[0]..&b[n-1] can be validly dereferenced). &b[-1], or
equivalently b-1, is not something we can use in valid Standard C.

gcc 6 diagnoses this as:

code/client/snd_wavelet.c:33:9: warning: array subscript is below array bounds [-Warray-bounds]

and might take this undefined behaviour as permission to emit
"more efficient" object code that is not what the author expected,
for example nothing at all. Use a macro to fake a 1-based array instead.
This function is used in the Team Arena menus

I don't think it's actually possible to reach this line with
foundPlayerServerNames < 1, because by the time we get here we have
set it to 1 + the actual number of servers; but if we did, it would
clearly underflow into foundPlayerServerNames[-1], which would be
undefined behaviour. gcc 6 diagnoses this with a warning:

code/ui/ui_main.c: In function ‘UI_BuildFindPlayerList’:
code/ui/ui_main.c:4138:16: warning: array subscript is below array bounds [-Warray-bounds]

Also correct the sizeof() invocation to make it more obviously
correct (in fact the buffers for names and addresses happen to both
be of size MAX_ADDRESSLENGTH, so it was fine, but it's good to be
obvious).
As with the other branch of the if/else, each element of
foundPlayerServerNames is in fact the same size as each element of
foundPlayerServerAddresses, so it was fine; but it's better to make
it obvious that we are using the right array sizes.
@timangus timangus merged commit f71fe6b into ioquake:master Sep 27, 2016
@smcv smcv deleted the underflow branch September 27, 2016 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants