Closed Bug 1381726 Opened 4 years ago Closed 4 years ago

Remove smprintf_free

Categories

(Core :: XPCOM, enhancement)

53 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: bzbarsky, Assigned: n.nethercote)

Details

Attachments

(2 files)

We should just be able to free() instead.
One obstacle is that comm-central has about 15 uses under mailnews/.
jorgk, this compiles locally.
Attachment #8889333 - Flags: review?(jorgk)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
We can just use free() instead.
Attachment #8889334 - Flags: review?(erahm)
Comment on attachment 8889333 [details] [diff] [review]
Replace nsTextFormatter::smprintf_free() uses with free()

Thanks for the heads-up!
Attachment #8889333 - Flags: review?(jorgk) → review+
Comment on attachment 8889334 [details] [diff] [review]
Remove nsTextFormatter::smprintf_free()

Review of attachment 8889334 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, another nice enhancement would be to have |smprintf| return a |UniquePtr| so we wouldn't have to worry about freeing at all.
Attachment #8889334 - Flags: review?(erahm) → review+
> Looks good, another nice enhancement would be to have |smprintf| return a
> |UniquePtr| so we wouldn't have to worry about freeing at all.

Yes, both JS_smprintf() and Smprintf() do that and I might do it in a follow-up.
jorgk: BTW, you should be able to land the c-c patch immediately without waiting for the other patch to land on m-c.
Flags: needinfo?(jorgk)
Thanks, I'm watching M-C merges anyway and I always need something to push after every M-C merge to trigger a full build.
Flags: needinfo?(jorgk)
Putting "leave-open" since our pulsebot would close it otherwise.
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/29d8abec1e2a
Replace nsTextFormatter::smprintf_free() uses with free(). r=jorgk
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/49b4b43ce3a7
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.