Skip to content

Fix error handling string memory leak#18

Open
jecassis wants to merge 1 commit intoprojectM-visualizer:masterfrom
jecassis:el
Open

Fix error handling string memory leak#18
jecassis wants to merge 1 commit intoprojectM-visualizer:masterfrom
jecassis:el

Conversation

@jecassis
Copy link
Copy Markdown

Through the use of the CRT debugger while developing foo_vis_milk2 I uncovered a small heap leak in the error handling which is not releasing the previous parser error string before storing a new one.

In "CompilerFunctions.c", prjm_eval_error() duplicates each parse error message with _strdup() and assigns it to cctx->error.error.
When multiple parse errors occur in the same compile context, each new assignment overwrites the previous pointer without freeing it first.

Here is the CRT output (on Windows):

Detected memory leaks!
Dumping objects ->
projectm-eval\CompilerFunctions.c(22) : {1428482} normal block at 0x000001E72F2BD2C0, 29 bytes long.
 Data: <syntax error, un> 73 79 6E 74 61 78 20 65 72 72 6F 72 2C 20 75 6E 
projectm-eval\CompilerFunctions.c(22) : {1408261} normal block at 0x000001E757B0CE00, 29 bytes long.
 Data: <syntax error, un> 73 79 6E 74 61 78 20 65 72 72 6F 72 2C 20 75 6E 

I tested the following fix:

void prjm_eval_error(PRJM_EVAL_LTYPE* loc, prjm_eval_compiler_context_t* cctx, yyscan_t yyscanner, char const* s)
{
    free(cctx->error.error);
    cctx->error.error = _strdup(s);
...

Before assigning a new error message, it frees the previous cctx->error.error in prjm_eval_error(). Consequently, each new parser error replaces the previous message without losing ownership of allocated memory.

free(NULL) is valid, so this is safe for first-use and unchanged behavior for successful parses.

This should result in no functional behavior delta and, since making this change, I have not seen the memory leaks reoccurring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant