Closed Bug 1197324 Opened 4 years ago Closed 3 years ago

add format-string checking attribute to snprintf_literal

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: froydnj, Assigned: evilpie)

References

Details

Attachments

(2 files)

As we start to use snprintf_literal more widely, it would be good to have format string checking enabled for it, so we don't do stupid things with it.
Assignee: nobody → evilpies
I wasn't sure if this should go into mozilla/Attributes.h so I just did the simple thing here.
Attachment #8777088 - Flags: review?(nfroyd)
Comment on attachment 8777088 [details] [diff] [review]
Format string checking for snprintf_literal

Review of attachment 8777088 [details] [diff] [review]:
-----------------------------------------------------------------

I doubt we have many cases for __attribute__((format)), so this is fine.

::: mfbt/Snprintf.h
@@ +17,5 @@
>  // function which uses template argument deduction to deduce the size of the
>  // buffer, avoiding the need for the user to pass it in explicitly.
>  #ifdef __cplusplus
>  template <size_t N>
> +#if defined(__GNUC__) || defined(__clang__)

FWIW, clang defines __GNUC__, so you are good to drop the __clang__ here.
Attachment #8777088 - Flags: review?(nfroyd) → review+
Comment on attachment 8777118 [details] [diff] [review]
Fix new -WFormat warnings

Would you be ok with reviewing this? Otherwise I probably need three reviewers.
Attachment #8777118 - Flags: review?(nfroyd)
Attachment #8777118 - Flags: review?(nfroyd) → review+
Pushed by evilpies@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0b04df0234b5
add format-string checking attribute to snprintf_literal. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/4b7261f08cce
Fix new warnings. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/0b04df0234b5
https://hg.mozilla.org/mozilla-central/rev/4b7261f08cce
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.