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

Reply via email to