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

Fix mxmlOptionsNew documentation #337

Closed
JohannesLorenz opened this issue Jan 18, 2025 · 3 comments
Closed

Fix mxmlOptionsNew documentation #337

JohannesLorenz opened this issue Jan 18, 2025 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@JohannesLorenz
Copy link

JohannesLorenz commented Jan 18, 2025

Hello,

first of all, I want to say thank you for providing MXML - we from ZynAddSubFX use it for more than a decade and have always been satisfied.

My request is concerning the cleanup of objects obtained through mxmlOptionsNew(). When I wrote our MXML4 port, I just checked the doc/mxml.html documentation of mxmlOptionsNew, and since it did not reference any cleanup function (the doxygen comment does, but this does not generate that html file), I assumed that the IO functions would cleanup that struct or some garbage collector behind the scenes. However, looking into MXML source, there is no garbage collector, and since there is an mxmlOptionsDelete() function, I assume a garbage collector is not planned.

I would like to request the following:

  1. In doc/mxml.html, in the mxmlOptionsNew function documentation, write that obtained pointers should be passed to mxmlOptionsDelete. I suggest to also write at what point of time that can be done - I assume this can be done as soon as the corresponding IO function finished, e.g. these IO functions never reference the object internally for later use.
  2. I also got confused because testmxml.c does not use mxmlOptionsDelete either. Do you mind adding it there?
  3. Minor issue I found: in doc/mxml.html, searching for "BACKTICK c" (just two characters, a backtick and then a c) shows that some code snippets are not rendered?

Thanks on advance!

@michaelrsweet michaelrsweet self-assigned this Jan 18, 2025
@michaelrsweet michaelrsweet added the documentation Improvements or additions to documentation label Jan 18, 2025
@michaelrsweet michaelrsweet added this to the Stable milestone Jan 18, 2025
@michaelrsweet
Copy link
Owner

Hmm, I generate the documentation with codedoc which should be including mxmlOptionsDelete... Yes, looking at the documentation it is there already. Also, the documentation for mxmlOptionsNew already references mxmlOptionsDelete.

However, it does look like the code example is messed up - probably a typo in the comments.

I can also add the mxmlOptionsDelete call to the test program but it isn't necessary when the program is exiting and returning all memory back to the OS.

@michaelrsweet michaelrsweet changed the title Reference and use mxmlOptionsDelete Fix mxmlOptionsNew documentation Jan 18, 2025
@JohannesLorenz
Copy link
Author

Indeed, I oversaw the reference to mxmlOptionsDelete in mxmlOptionsNew. That solves point 1.

I can also add the mxmlOptionsDelete call to the test program but it isn't necessary when the program is exiting and returning all memory back to the OS.

That would help. Some applications have the goal to be completely leak-free.

@michaelrsweet
Copy link
Owner

[master b44e7de] Fix documentation and add usage of mxmlOptionsDelete in test program (Issue #337)

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

No branches or pull requests

2 participants