Closed
Bug 1302233
Opened 8 years ago
Closed 8 years ago
Introduce VsprintfLiteral to mfbt
Categories
(Core :: MFBT, defect)
Core
MFBT
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: evilpie, Assigned: evilpie)
References
Details
Attachments
(1 file)
837 bytes,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
This variant can be used to replace vsnprintf.
Attachment #8790448 -
Flags: review?(nfroyd)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → evilpies
Comment 1•8 years ago
|
||
Comment on attachment 8790448 [details] [diff] [review]
Introduce VsprintfLiteral to mfbt
Review of attachment 8790448 [details] [diff] [review]:
-----------------------------------------------------------------
::: mfbt/Sprintf.h
@@ +16,5 @@
> +
> +template <size_t N>
> +int VsprintfLiteral(char (&buffer)[N], const char* format, va_list args)
> +{
> + int result = vsnprintf(buffer, N, format, args);
Maybe MOZ_ASSERT(format != buffer) and MOZ_ASSERT(result >= 0) as sanity checks?
Comment 2•8 years ago
|
||
Comment on attachment 8790448 [details] [diff] [review]
Introduce VsprintfLiteral to mfbt
Review of attachment 8790448 [details] [diff] [review]:
-----------------------------------------------------------------
I think Chris's comment to MOZ_ASSERT(format != buffer) is a reasonable sanity check. I'm less sure about MOZ_ASSERT(result >= 0); it seems like we ought to be leaving that decision up to the callers. Then again, the man page for snprintf(3) says that it'll only return < 0 if "an output error" occurred, which seems highly unlikely to happen with snprintf et al.
Attachment #8790448 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 3•8 years ago
|
||
I am relatively sure snprintf < 0 shouldn't happen with a fixed buffer, but there must be so many a lot of implementations across different platforms and it doesn't say this explicitly anywhere I looked. I am going to leave that out. MOZ_ASSERT(format != buffer) is a good idea.
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bb1a3059c91
Introduce VsprintfLiteral to mfbt. r=froydnj
Comment 5•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in
before you can comment on or make changes to this bug.
Description
•