Closed
Bug 1381726
Opened 7 years ago
Closed 7 years ago
Remove smprintf_free
Categories
(Core :: XPCOM, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox56 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: n.nethercote)
Details
Attachments
(2 files)
10.46 KB,
patch
|
jorgk-bmo
:
review+
|
Details | Diff | Splinter Review |
4.02 KB,
patch
|
erahm
:
review+
|
Details | Diff | Splinter Review |
We should just be able to free() instead.
Assignee | ||
Comment 1•7 years ago
|
||
One obstacle is that comm-central has about 15 uses under mailnews/.
Assignee | ||
Comment 2•7 years ago
|
||
jorgk, this compiles locally.
Attachment #8889333 -
Flags: review?(jorgk)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•7 years ago
|
||
We can just use free() instead.
Attachment #8889334 -
Flags: review?(erahm)
Comment 4•7 years ago
|
||
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 5•7 years ago
|
||
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+
Assignee | ||
Comment 6•7 years ago
|
||
> 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.
Assignee | ||
Comment 7•7 years ago
|
||
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)
Comment 8•7 years ago
|
||
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)
Assignee | ||
Comment 9•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/49b4b43ce3a7b559fd7b33b0fe3c171b72c2cceb Bug 1381726 - Remove nsTextFormatter::smprintf_free(). r=erahm.
Comment 10•7 years ago
|
||
Putting "leave-open" since our pulsebot would close it otherwise.
Keywords: leave-open
Comment 11•7 years ago
|
||
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/29d8abec1e2a Replace nsTextFormatter::smprintf_free() uses with free(). r=jorgk
Updated•7 years ago
|
Keywords: leave-open
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/49b4b43ce3a7
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in
before you can comment on or make changes to this bug.
Description
•