On Mon, 14 Oct 2024 at 17:37, Michael Orlitzky <mich...@orlitzky.com> wrote: > > On Mon, 2023-11-20 at 12:27 -0500, Michael Orlitzky wrote:
> > > > https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=shortlog;h=refs/heads/libcss > Any chance I can rekindle the interest in this? I'm still around to > support the patchset and there are no known bugs. Sorry it's taken so long. I've finally had another look. https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny_css.c;h=96991ddfcfcfe4684b9411709c44367b4651fa92;hb=refs/heads/libcss#l1929 In `svgtiny_dom_user_data_handler` str is leaked. There is a comment there is a comment there that says that there is no way to access the `state->interned_userdata_key`. There are two options here: 1. We could change the `dom_node_set_user_data` function to take a `void *pw` alongside the `handler` callback, and change the hander prototype to add an argument to pass back the user `pw`. 2. We can simply drop the string comparison here. The only user data we add that uses that callback is the libcss_node_data. If we added any other node data, we could add alongside a different callback function. There's also a comment saying: > dom_node_set_user_data() always returns DOM_NO_ERR However, it can return `DOM_NO_MEM_ERR` when malloc fails. In general there are a lot of ignored return values in the patches. https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny.c;h=9a6709a7e0069aad5290d43ebad3dcc9067d58a3;hb=refs/heads/libcss#l978 In `svgtiny_parse_svg`, it calls `svgtiny_parse_styles` which returns the css_select_results. These are leaked on all the error paths in `svgtiny_parse_svg` and I think at least one of the early returns is missing a call to `svgtiny_cleanup_state_local`. https://gitweb.michael.orlitzky.com/?p=libsvgtiny.git;a=blob;f=src/svgtiny.c;h=9a6709a7e0069aad5290d43ebad3dcc9067d58a3;hb=refs/heads/libcss#l812 In `svgtiny_parse_style_element`, the `svgtiny_create_stylesheet` function is called, which returns a css_stylesheet pointer. Data is added to the sheet and then the sheet is given to `css_select_ctx_append_sheet`. However, `css_select_ctx_append_sheet` does not take ownership of the sheet (it takes it with the const qualifier). So the sheet is leaked at the end of `svgtiny_parse_style_element`. > Long term, I think using libcss is a better approach for parsing all of > the CSS properties that now live inside the SVG spec. Agreed! Thanks for working on this, and sorry it's taken me so long to look at it! Michael