Closed Bug 1334278 Opened 8 years ago Closed 8 years ago

Smprintf should return a UniquePtr

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: tromey, Assigned: tromey)

References

Details

Attachments

(3 files)

Bug 1060419 introduces Smprintf, which returns a newly allocated buffer, which must be freed with SmprintfFree. This API was chosen there to minimize churn while converting NSPR and JS_* code. However, it would be cleaner for this to return a UniquePtr.
Note that then SmprintfFree could be removed, and also the SmprintfFreePolicy in xpcom/components/ManifestParser.cpp as well.
Continuing my printf quest.
Assignee: nobody → ttromey
Comment on attachment 8860536 [details] Bug 1334278 - change mozilla::Smprintf to return a UniquePtr; https://reviewboard.mozilla.org/r/132554/#review135874 This is great, thank you!
Attachment #8860536 - Flags: review?(nfroyd) → review+
Attachment #8860537 - Flags: review?(nfroyd) → review+
Attachment #8860538 - Flags: review?(nfroyd) → review+
I must have made some change before requesting review, because just look at that terrible try run. Ugh. Anyway, new version, actually works. Will push to try momentarily. I'm going to ask for re-review of one patch because I changed initBytes to take a UniqueChars&&, which I think makes the ownership transfer much more clear.
Comment on attachment 8860537 [details] Bug 1334278 - change JS_smprintf to return UniqueChars; I think this could use a re-review, see the initBytes change. I made this change to make the ownership transfer more clear.
Attachment #8860537 - Flags: review+ → review?(nfroyd)
Comment on attachment 8860537 [details] Bug 1334278 - change JS_smprintf to return UniqueChars; https://reviewboard.mozilla.org/r/132556/#review136358 I like this changes in this version. Down with raw pointer parameters!
Attachment #8860537 - Flags: review?(nfroyd) → review+
Looks like the usual unified build problem. I'll upload a new version in a bit.
The nu build hates me.
Rebased.
Pushed by ttromey@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/815a95113981 change mozilla::Smprintf to return a UniquePtr; r=froydnj https://hg.mozilla.org/integration/autoland/rev/ea31640ea9a3 change JS_smprintf to return UniqueChars; r=froydnj https://hg.mozilla.org/integration/autoland/rev/ae4e80fe5f34 have FormatStackDump return UniqueChars; r=froydnj
Depends on: 1360152
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: