If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

gfxHarfBuzzShaper::InitTextRun leaks hb_font_funcs_create() and hb_unicode_funcs_create()

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

({coverity, mlk})

Trunk
coverity, mlk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

1.42 KB, patch
jfkthame
: review+
Joe Drew (not getting mail)
: approval2.0+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
689 gfxHarfBuzzShaper::InitTextRun(gfxContext *aContext,

not sure about this one yet:
705             sHBFontFuncs = hb_font_funcs_copy(hb_font_funcs_create());

713             sHBUnicodeFuncs = hb_unicode_funcs_copy(hb_unicode_funcs_create());

this is more or less a standard allocator, it creates a structure and returns a pointer to it:
91 hb_font_funcs_t *
92 hb_font_funcs_create (void)
93 {
94   hb_font_funcs_t *ffuncs;
95 
96   if (!HB_OBJECT_DO_CREATE (hb_font_funcs_t, ffuncs))
97     return &_hb_font_funcs_nil;
98 
99   ffuncs->v = _hb_font_funcs_nil.v;
100 
101   return ffuncs;
102 }

this is more or less a standard allocator, it creates a structure and returns a pointer to it:
53 hb_unicode_funcs_t *
54 hb_unicode_funcs_create (void)
55 {
56   hb_unicode_funcs_t *ufuncs;
57 
58   if (!HB_OBJECT_DO_CREATE (hb_unicode_funcs_t, ufuncs))
59     return &_hb_unicode_funcs_nil;
60 
61   ufuncs->v = _hb_unicode_funcs_nil.v;
62 
63   return ufuncs;
64 }

this takes a pointer, copies bits from its referenced structure, but does not retain a reference to the structure (thus if no one else does, that data pointed to by the pointer is leaked):
124 hb_font_funcs_t *
125 hb_font_funcs_copy (hb_font_funcs_t *other_ffuncs)
126 {
127   hb_font_funcs_t *ffuncs;
128 
129   if (!HB_OBJECT_DO_CREATE (hb_font_funcs_t, ffuncs))
130     return &_hb_font_funcs_nil;
131 
132   ffuncs->v = other_ffuncs->v;
133 
134   return ffuncs;
135 }

this takes a pointer, copies bits from its referenced structure, but does not retain a reference to the structure (thus if no one else does, that data pointed to by the pointer is leaked):
87 hb_unicode_funcs_copy (hb_unicode_funcs_t *other_ufuncs)
88 {
89   hb_unicode_funcs_t *ufuncs;
90 
91   if (!HB_OBJECT_DO_CREATE (hb_unicode_funcs_t, ufuncs))
92     return &_hb_unicode_funcs_nil;
93 
94   ufuncs->v = other_ufuncs->v;
95 
96   return ufuncs;
97 }
(Assignee)

Comment 1

7 years ago
Created attachment 482493 [details] [diff] [review]
proposal
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #482493 - Flags: review?(jfkthame)
Comment on attachment 482493 [details] [diff] [review]
proposal

Hmm, yes... looks like the hb_*_copy() calls are redundant here.

(They're left over from working with older versions of the harfbuzz code in which the hb_*_create() calls returned immutable objects, and it was necessary to copy() them in order to get modifiable ones. But that problem seems to be gone.)
Attachment #482493 - Flags: review?(jfkthame) → review+
(Assignee)

Updated

7 years ago
Attachment #482493 - Flags: approval2.0?
Attachment #482493 - Flags: approval2.0? → approval2.0+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/7c282eb45b9b
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.