Dear Jonathan, > AFAICS, the patch will leak the graphite2_funcs_t record that's attached > to the face, as it fails to free it in > _hb_graphite2_shaper_face_data_destroy.
Thanks for flagging that. I've restructured grfuncs to be part of the shaper_data so there is no m/calloc to free now. It has the same lifetime and handling as the overall shaper_data > > (It also fails to free it if _hb_graphite2_shaper_face_data_create hits > an error in gr_make_face, or if hb_graphite2_load_gr fails to find one > of the expected functions in the library.) A second patch fixes the dlhandle leak if gr_make_face fails. > I wonder if it'd be better to ALWAYS do the dynamic-load thing, and > scrap the HAVE_GRAPHITE2_STATIC option? This would substantially clean > up the #if-clutter that currently makes things look a bit hairy, and > probably make it easier to verify that the code paths are all sane. I'm undecided on that. Users may still want the option of direct linking? > As for whether to do this in general -- I think that if we can ensure > the code is clean enough that it won't introduce new leaks (see above) > or vulnerabilities, it'd provide a crucial feature that's currently > lacking for most client apps. In Gecko, we don't need this as we have a > separate Graphite codepath that's independent of harfbuzz (though we > could consider changing that some day), but for software that uses a > harfbuzz rendering path exclusively, this could offer a valuable added > capability. Yours, Martin _______________________________________________ HarfBuzz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/harfbuzz
