Static code check findings -- Possible memory leak in realloc

[laszloh]

Hi,
cppcheck (static code analysis tool) found 3 findings with rating high/error:

src\SdCard.cpp:162: [high:error] Common realloc mistake: 'subdirectoryList' nulled but not freed upon failure [memleakOnRealloc]
src\SdCard.cpp:305: [high:error] Common realloc mistake: 'serializedPlaylist' nulled but not freed upon failure [memleakOnRealloc]
src\SdCard.cpp:391: [high:error] Common realloc mistake: 'serializedPlaylist' nulled but not freed upon failure [memleakOnRealloc]

The problem stems from the working of realloc. If it fails, the original memory block is not modified and it returns NULL. The code here copies the pointer directly into the "old" pointer, so if realloc returns NULL the reference to the original memory address is lost and it can not be freed any more on an error.

To use realloc safely, a temporary variable has to be used. F.e. like this:

char *buf = (char*)malloc(A_SIZE);

// realloc using a temporary pointer
char *tmp = (char*)realloc(buf, A_NEW_SIZE);
if(tmp==NULL) {
    // realloc failed do some error handling and clean up
    free(buf);
    return NULL;
}
// realloc worked, so update original pointer
buf = tmp;

edit:
We also seem to have a similar leak-problem at:

ESPuino/src/AudioPlayer.cpp

Line 871 in f4f1e29

filename = SdCard_pickRandomSubdirectory(filename); // *filename (input): target-directory // *filename (output): random subdirectory

Here we malloc into filename, but SdCard_pickRandomSubdirectory can return NULL if something goes wrong. Then filename can not be freed any more.