[cppcheck] Improvements in SaveStorageValue() in core.c (#1160)

* [cppcheck] Improvements in SaveStorageValue() in core.c

in file core.c cppcheck shows errors only in function SaveStorageValue():

* Common realloc mistake: 'fileData' nulled but not freed upon failure
* Memory pointed to by 'fileData' is freed twice.

Validation:
* Tested examples/core/core_storage_values.c
* Launched Unit Test for this function
* Rerun CPPCHECK afer fix

* [cppcheck] Change functions header to accept only positive position in files

Changes:
* Functions SaveStorageValue(), LoadStorageValue() (core.c)
* Functions LoadFileData(), SaveFileData() (utils.c)
* Headers in raylib.h

Validation:
* Tested examples/core/core_storage_values.c
* Launched Unit Test for these functions
* Rerun CPPCHECK afer fix
This commit is contained in:
Dani Martin 2020-03-30 13:51:36 +02:00 committed by GitHub
parent 9b66883e0b
commit 62cdb2299b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 42 additions and 20 deletions

View File

@ -16,7 +16,7 @@
* *
* #define PLATFORM_DESKTOP * #define PLATFORM_DESKTOP
* Windowing and input system configured for desktop platforms: Windows, Linux, OSX, FreeBSD, OpenBSD, NetBSD, DragonFly * Windowing and input system configured for desktop platforms: Windows, Linux, OSX, FreeBSD, OpenBSD, NetBSD, DragonFly
* NOTE: Oculus Rift CV1 requires PLATFORM_DESKTOP for mirror rendering - View [rlgl] module to enable it * NOTE: Oculus Ritf CV1 requires PLATFORM_DESKTOP for mirror rendering - View [rlgl] module to enable it
* *
* #define PLATFORM_ANDROID * #define PLATFORM_ANDROID
* Windowing and input system configured for Android device, app activity managed internally in this module. * Windowing and input system configured for Android device, app activity managed internally in this module.
@ -2192,7 +2192,7 @@ unsigned char *DecompressData(unsigned char *compData, int compDataLength, int *
// Save integer value to storage file (to defined position) // Save integer value to storage file (to defined position)
// NOTE: Storage positions is directly related to file memory layout (4 bytes each integer) // NOTE: Storage positions is directly related to file memory layout (4 bytes each integer)
void SaveStorageValue(int position, int value) void SaveStorageValue(unsigned int position, int value)
{ {
#if defined(SUPPORT_DATA_STORAGE) #if defined(SUPPORT_DATA_STORAGE)
char path[512] = { 0 }; char path[512] = { 0 };
@ -2204,31 +2204,53 @@ void SaveStorageValue(int position, int value)
strcpy(path, STORAGE_DATA_FILE); strcpy(path, STORAGE_DATA_FILE);
#endif #endif
int dataSize = 0; unsigned int dataSize = 0;
unsigned int newDataSize = 0;
unsigned char *fileData = LoadFileData(path, &dataSize); unsigned char *fileData = LoadFileData(path, &dataSize);
unsigned char *newFileData = NULL;
if (fileData != NULL) if (fileData != NULL)
{ {
if (dataSize <= (position*sizeof(int))) if (dataSize <= (position*sizeof(int)))
{ {
// Increase data size up to position and store value // Increase data size up to position and store value
dataSize = (position + 1)*sizeof(int); newDataSize = (position + 1)*sizeof(int);
fileData = (unsigned char *)RL_REALLOC(fileData, dataSize); newFileData = (unsigned char *)RL_REALLOC(fileData, newDataSize);
int *dataPtr = (int *)fileData;
dataPtr[position] = value; if (newFileData != NULL)
{
// RL_REALLOC succeded
int *dataPtr = (int *)newFileData;
dataPtr[position] = value;
}
else
{
// RL_REALLOC failed
TRACELOG(LOG_WARNING, "FILEIO: Position in bytes (%u) bigger than actual size of file [%s] (%u) Realloc function FAIL",position*sizeof(int),path,dataSize);
// We store the old size of the file.
newFileData=fileData;
newDataSize=dataSize;
}
} }
else else
{ {
// We store the old size of the file.
newFileData=fileData;
newDataSize=dataSize;
// Replace value on selected position // Replace value on selected position
int *dataPtr = (int *)fileData; int *dataPtr = (int *)newFileData;
dataPtr[position] = value; dataPtr[position] = value;
} }
SaveFileData(path, fileData, dataSize); SaveFileData(path, newFileData, newDataSize);
RL_FREE(fileData); RL_FREE(newFileData);
} }
else else
{ {
TRACELOG(LOG_INFO, "FILEIO: [%s] File not found, creating it.",path);
dataSize = (position + 1)*sizeof(int); dataSize = (position + 1)*sizeof(int);
fileData = (unsigned char *)RL_MALLOC(dataSize); fileData = (unsigned char *)RL_MALLOC(dataSize);
int *dataPtr = (int *)fileData; int *dataPtr = (int *)fileData;
@ -2242,7 +2264,7 @@ void SaveStorageValue(int position, int value)
// Load integer value from storage file (from defined position) // Load integer value from storage file (from defined position)
// NOTE: If requested position could not be found, value 0 is returned // NOTE: If requested position could not be found, value 0 is returned
int LoadStorageValue(int position) int LoadStorageValue(unsigned int position)
{ {
int value = 0; int value = 0;
#if defined(SUPPORT_DATA_STORAGE) #if defined(SUPPORT_DATA_STORAGE)
@ -2255,7 +2277,7 @@ int LoadStorageValue(int position)
strcpy(path, STORAGE_DATA_FILE); strcpy(path, STORAGE_DATA_FILE);
#endif #endif
int dataSize = 0; unsigned int dataSize = 0;
unsigned char *fileData = LoadFileData(path, &dataSize); unsigned char *fileData = LoadFileData(path, &dataSize);
if (fileData != NULL) if (fileData != NULL)

View File

@ -951,8 +951,8 @@ RLAPI void TakeScreenshot(const char *fileName); // Takes a scr
RLAPI int GetRandomValue(int min, int max); // Returns a random value between min and max (both included) RLAPI int GetRandomValue(int min, int max); // Returns a random value between min and max (both included)
// Files management functions // Files management functions
RLAPI unsigned char *LoadFileData(const char *fileName, int *bytesRead); // Load file data as byte array (read) RLAPI unsigned char *LoadFileData(const char *fileName, unsigned int *bytesRead); // Load file data as byte array (read)
RLAPI void SaveFileData(const char *fileName, void *data, int bytesToWrite); // Save data to file from byte array (write) RLAPI void SaveFileData(const char *fileName, void *data, unsigned int bytesToWrite); // Save data to file from byte array (write)
RLAPI char *LoadFileText(const char *fileName); // Load text data from file (read), returns a '\0' terminated string RLAPI char *LoadFileText(const char *fileName); // Load text data from file (read), returns a '\0' terminated string
RLAPI void SaveFileText(const char *fileName, char *text); // Save text data to file (write), string must be '\0' terminated RLAPI void SaveFileText(const char *fileName, char *text); // Save text data to file (write), string must be '\0' terminated
RLAPI bool FileExists(const char *fileName); // Check if file exists RLAPI bool FileExists(const char *fileName); // Check if file exists
@ -976,8 +976,8 @@ RLAPI unsigned char *CompressData(unsigned char *data, int dataLength, int *comp
RLAPI unsigned char *DecompressData(unsigned char *compData, int compDataLength, int *dataLength); // Decompress data (DEFLATE algorythm) RLAPI unsigned char *DecompressData(unsigned char *compData, int compDataLength, int *dataLength); // Decompress data (DEFLATE algorythm)
// Persistent storage management // Persistent storage management
RLAPI void SaveStorageValue(int position, int value); // Save integer value to storage file (to defined position) RLAPI void SaveStorageValue(unsigned int position, int value); // Save integer value to storage file (to defined position)
RLAPI int LoadStorageValue(int position); // Load integer value from storage file (from defined position) RLAPI int LoadStorageValue(unsigned int position); // Load integer value from storage file (from defined position)
RLAPI void OpenURL(const char *url); // Open URL with default system browser (if available) RLAPI void OpenURL(const char *url); // Open URL with default system browser (if available)

View File

@ -165,7 +165,7 @@ void TraceLog(int logType, const char *text, ...)
} }
// Load data from file into a buffer // Load data from file into a buffer
unsigned char *LoadFileData(const char *fileName, int *bytesRead) unsigned char *LoadFileData(const char *fileName, unsigned int *bytesRead)
{ {
unsigned char *data = NULL; unsigned char *data = NULL;
*bytesRead = 0; *bytesRead = 0;
@ -187,7 +187,7 @@ unsigned char *LoadFileData(const char *fileName, int *bytesRead)
data = (unsigned char *)RL_MALLOC(sizeof(unsigned char)*size); data = (unsigned char *)RL_MALLOC(sizeof(unsigned char)*size);
// NOTE: fread() returns number of read elements instead of bytes, so we read [1 byte, size elements] // NOTE: fread() returns number of read elements instead of bytes, so we read [1 byte, size elements]
int count = fread(data, sizeof(unsigned char), size, file); unsigned int count = fread(data, sizeof(unsigned char), size, file);
*bytesRead = count; *bytesRead = count;
if (count != size) TRACELOG(LOG_WARNING, "FILEIO: [%s] File partially loaded", fileName); if (count != size) TRACELOG(LOG_WARNING, "FILEIO: [%s] File partially loaded", fileName);
@ -205,7 +205,7 @@ unsigned char *LoadFileData(const char *fileName, int *bytesRead)
} }
// Save data to file from buffer // Save data to file from buffer
void SaveFileData(const char *fileName, void *data, int bytesToWrite) void SaveFileData(const char *fileName, void *data, unsigned int bytesToWrite)
{ {
if (fileName != NULL) if (fileName != NULL)
{ {
@ -213,7 +213,7 @@ void SaveFileData(const char *fileName, void *data, int bytesToWrite)
if (file != NULL) if (file != NULL)
{ {
int count = fwrite(data, sizeof(unsigned char), bytesToWrite, file); unsigned int count = fwrite(data, sizeof(unsigned char), bytesToWrite, file);
if (count == 0) TRACELOG(LOG_WARNING, "FILEIO: [%s] Failed to write file", fileName); if (count == 0) TRACELOG(LOG_WARNING, "FILEIO: [%s] Failed to write file", fileName);
else if (count != bytesToWrite) TRACELOG(LOG_WARNING, "FILEIO: [%s] File partially written", fileName); else if (count != bytesToWrite) TRACELOG(LOG_WARNING, "FILEIO: [%s] File partially written", fileName);