Introduce VsprintfLiteral to mfbt

RESOLVED FIXED in Firefox 51

Status

()

Core
MFBT
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

unspecified
mozilla51
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

a year ago
Created attachment 8790448 [details] [diff] [review]
Introduce VsprintfLiteral to mfbt

This variant can be used to replace vsnprintf.
Attachment #8790448 - Flags: review?(nfroyd)
(Assignee)

Updated

a year ago
Assignee: nobody → evilpies
(Assignee)

Updated

a year ago
Blocks: 1302243
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 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

a year 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.

Comment 4

a year ago
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7bb1a3059c91
Introduce VsprintfLiteral to mfbt. r=froydnj

Comment 5

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7bb1a3059c91
Status: NEW → RESOLVED
Last Resolved: a year 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.