Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
morgant opened this issue Aug 24, 2024 · 0 comments
Assignees
Milestone

Comments

@morgant
Copy link
Owner

morgant commented Aug 24, 2024

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.

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

No branches or pull requests

1 participant