Closed Bug 1302233 Opened 3 years ago Closed 3 years ago

Introduce VsprintfLiteral to mfbt

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: evilpie, Assigned: evilpie)

References

Details

Attachments

(1 file)

This variant can be used to replace vsnprintf.
Attachment #8790448 - Flags: review?(nfroyd)
Assignee: nobody → evilpies
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+
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
https://hg.mozilla.org/mozilla-central/rev/7bb1a3059c91
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.