Fix some warnings #984
Fix some warnings #984
Conversation
th-otto
force-pushed
the
PR-37
branch
2 times, most recently
from
bc5661c to
fc42152
Compare
giulianobelinassi
left a comment
giulianobelinassi
left a comment
There was a problem hiding this comment.
Please take a look at my comments.
| } | ||
| bool Is_Valid(void) const | ||
| { | ||
| return (this != NULL && NextNode != NULL && PrevNode != NULL); |
There was a problem hiding this comment.
This actually seems to be something intended by the original programmers rather than a programming error. Are you sure this is actually not necessary?
For example, if you call ->Is_Valid() with a nullptr object this function will return false rather than crashing the code.
| // | ||
| memcpy(field, curbuf, FIELD_HEADER_SIZE); | ||
| curbuf += FIELD_HEADER_SIZE; | ||
| memcpy(field->ID, curbuf, 4); |
There was a problem hiding this comment.
This should take the sizeof(field->ID) rather than hardcoding it to 4. If we decide later to change this in the future this memcpy would be very error prone.
|
|
||
| memcpy(&Palette[0], &palette.Palette[0], sizeof(Palette)); | ||
| for (int i = 0; i < PaletteClass::COLOR_COUNT; i++) | ||
| Palette[i] = palette.Palette[i]; |
There was a problem hiding this comment.
This is replacing a very optimized copy of palettes with a potentially non-optimized one. Are you sure this is necessary?
| SeedBits = 0; | ||
| Current = 0; | ||
| memset(Random, '\0', sizeof(Random)); | ||
| for (size_t i = 0; i < sizeof(Random) / sizeof(Random[0]); i++) { |
There was a problem hiding this comment.
use ARRAY_SIZE, defined in common/macros.h
| void RandomStraw::Seed_Short(short seed) | ||
| { | ||
| for (int index = 0; index < (sizeof(seed) * CHAR_BIT); index++) { | ||
| for (int index = 0; index < ((int)sizeof(seed) * CHAR_BIT); index++) { |
There was a problem hiding this comment.
size_t may be cleaner than int here
| } | ||
|
|
||
| for (int i = 0; i < strlen(path); i++) { | ||
| for (int i = 0; i < (int)strlen(path); i++) { |
There was a problem hiding this comment.
Avoid calling strlen in a loop comparison like this. This is O(n²) instead of O(n). Instead store the size of the string into a variable and use that instead.
2 participants
No description provided.