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
| @@ -107,7 +107,7 @@ class GenericNode | |||
| } | |||
| 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.
| @@ -131,8 +131,12 @@ PacketClass::PacketClass(char* curbuf) | |||
| // | |||
| // Copy the adjusted header into the buffer and then advance the buffer | |||
| // | |||
| 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.
| @@ -118,7 +118,8 @@ PaletteClass& PaletteClass::operator=(PaletteClass const& palette) | |||
| if (this == &palette) | |||
| return (*this); | |||
|
|
|||
| 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?
| @@ -108,7 +108,9 @@ void RandomStraw::Reset(void) | |||
| { | |||
| 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
| @@ -210,7 +212,7 @@ void RandomStraw::Seed_Byte(char seed) | |||
| *=============================================================================================*/ | |||
| 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
| @@ -250,7 +250,7 @@ inline static void _splitpath(const char* path, char* drive, char* dir, char* fn | |||
| return; | |||
| } | |||
|
|
|||
| 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.
No description provided.