Results from the infer static analyser

[Joshua Moerman]

I ran the static analysis tool infer. I already opened a pull request fixing two issues I could fix myself. Besides many false alarms there are two issues remaining:

First issue:

src/rdb.c:1040: error: USE_AFTER_FREE

  pointer `fp` last assigned on line 994 was freed by call to `fclose()` at line 1015, column 9 and is dereferenced or freed at line 1040, column 5

  1038.   werr:

  1039.       serverLog(LL_WARNING,"Write error saving DB on disk: %s", strerror(errno));

  1040. >     fclose(fp);

  1041.       unlink(tmpfile);

  1042.       return C_ERR;

What happens here: the return value of fclose is checked earlier in the function and jumps to werr if it fails, then fcloseing it again. Doing an fclose twice is undefined behaviour. (I can't imagine this actually happens, but infer is right here.)

The second issue has to do with malloc. Infer assumes the return value of malloc can be NULL (and hence has to be checked). This is not checked throughout the codebase. I am not sure what to say about this, also because most implementations of malloc actually never return NULL (even when the memory is full).

[Joshua Moerman]

I realise that I should say more on the second point: It could be that there are other null pointer dereference issues which were overshadowed by the vast amount of issues with malloc. (I simply ignored all null pointer dereferences, because the list was too big, and didn't seem interesting.) I will try to extract those, when I have the time.