mlvwm icon indicating copy to clipboard operation
mlvwm copied to clipboard

`SetStartFunction()` in `config.c` doesn't free memory allocated for `ShortCuts` in prior executions

Open morgant opened this issue 1 year ago • 0 comments

While testing to see if one could use multiple InitFunction/RestartFunction built-in function blocks in mlvwm config files for the purpose of appending commands/lines later (especially when Read-ing additional configuration files; see the Set default desktop pattern in themes in such a way that they can be overridden in the mlvwmrc project), I found a memory leak:

SetStartFunction() in mlvwm/config.c initializes the new variable as a reference to Scr.StartFunc (new = &Scr.StartFunc;), then the first iteration of the while loop allocates memory for a new ShortCut into new/Scr.StartFunc (*new = calloc( 1, sizeof( ShortCut ) );). Subsequent iterations save a reference to new in the prev variable and associate them as a linked list.

If SetStartFunction(), which is executed for the InitFunction/RestartFunction built-in functions, is called several times, the previously allocated memory for the linked list of ShortCuts will not be freed and the head of the linked list will be replaced. So, this is leaking memory.

I see several options for resolving this and addressing the want to append InitFunction/RestartFunction configuration block contents:

  1. Fix SetStartFunction to free the previously allocated linked list if Scr.StartFunc is not null, then declare/document that InitFunction/RestartFunction built-in function blocks cannot append, only create/replace
  2. Add new built-in function(s) for destroying/freeing the previously allocated linked list if Scr.StartFunc is not null (maybe DestroyInitFunction/DestroyRestartFunction?), then declare/document that InitFunction/RestartFunction create or append
  3. Add new built-in functions for appending to (maybe AppendToInitFunction/AppendToRestartFunction?) and destroying/freeing (again, maybe DestroyInitFunctionDestroy/DestroyRestartFunction?) the previously allocated Scr.StartFunc linked list, then declare/document that: InitFunction/RestartFunction only create, AppendToInitFunction/AppendToRestartFunction create or append, and DestroyInitFunction/DestroyRestartFunction delete

I did look at the fvwm 2 manual page when preparing the proposed solutions. I'm leaning toward the second (InitFunction/RestartFunction create or append and add explicit DestroyInitFunction/DestroyRestartFunction built-in functions) or third (separate & explicit create/append/destroy built-in functions) options as that would allow more customization, especially in the mlvwmrc project's implementation.

morgant avatar Aug 24 '24 16:08 morgant