[HarfBuzz] Loading Graphite dynamically

[Martin Hosken]

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