Smprintf should return a UniquePtr

RESOLVED FIXED in Firefox 55

Status

()

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: tromey, Assigned: tromey)

Tracking

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(3 attachments)

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+
Comment on attachment 8860537 [details]
Bug 1334278 - change JS_smprintf to return UniqueChars;

https://reviewboard.mozilla.org/r/132556/#review135876
Attachment #8860537 - Flags: review?(nfroyd) → review+
Comment on attachment 8860538 [details]
Bug 1334278 - have FormatStackDump return UniqueChars;

https://reviewboard.mozilla.org/r/132558/#review135878
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.