1197316 - remove PR_snprintf calls in xpcom/

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

Steps:

1. Find all calls to PR_snprintf in xpcom/.
2. Add #include "mozilla/Snprintf.h" to the file(s).
3. Replace PR_snprintf(X, sizeof(X), ...) calls with snprintf_literal(X, ...).
4. Replace other PR_snprintf calls with snprintf.
5. Remove any #include "prprf.h" lines.

Comment 1

[Miko Mynttinen]

Attachment: Remove PR_snprintf calls in xpcom/

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8651416 [details] [diff] [review]
Remove PR_snprintf calls in xpcom/

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

Thanks for the patch!  I apologize for forgetting to mention that the format strings would have to be double-checked to make sure they are specifying the correct thing, so there's a lot of those kind of corrections below.

::: xpcom/ds/nsSupportsPrimitives.cpp
@@ +5,5 @@
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "nsSupportsPrimitives.h"
>  #include "nsMemory.h"
> +#include "mozilla/Snprintf.h"

Given all the format string fixups below, you'll also have to add:

#include "mozilla/IntegerPrintfMacros.h"

here.  Please add it before the #include of Snprintf.h.  (If you're wondering why we don't just use <inttypes.h>, please see that header for more information.)

@@ +239,5 @@
>  nsSupportsPRUint8Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[8];
> +  snprintf_literal(buf, "%u", (uint16_t)mData);

I think this should be |(unsigned int)mData|.

@@ +284,5 @@
>  nsSupportsPRUint16Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[8];
> +  snprintf_literal(buf, "%u", (int)mData);

Likewise here.

@@ +329,5 @@
>  nsSupportsPRUint32Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[16];
> +  snprintf_literal(buf, "%lu", mData);

This format string should be "%u".

@@ +374,5 @@
>  nsSupportsPRUint64Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[32];
> +  snprintf_literal(buf, "%llu", mData);

The format string here should be |"%" PRIu64|.

@@ +512,5 @@
>  nsSupportsPRInt16Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[8];
> +  snprintf_literal(buf, "%d", mData);

We need this to be |(int)mData|.

@@ +557,5 @@
>  nsSupportsPRInt32Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[16];
> +  snprintf_literal(buf, "%ld", mData);

The format string here should be "%d".

@@ +601,5 @@
>  NS_IMETHODIMP
>  nsSupportsPRInt64Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[16];

Please change this to 32, as it was before, since the largest 64-bit integer will take more than 16 characters.

@@ +602,5 @@
>  nsSupportsPRInt64Impl::ToString(char** aResult)
>  {
>    NS_ASSERTION(aResult, "Bad pointer");
> +  char buf[16];
> +  snprintf_literal(buf, "%lld", mData);

The format string here should be |"%" PRId64|.

Comment 3

[Miko Mynttinen]

Attachment: Remove PR_snprintf calls in xpcom/

Comment 4

[Miko Mynttinen]

(In reply to Nathan Froyd [:froydnj][:nfroyd] from comment #2)

> Thanks for the patch!  I apologize for forgetting to mention that the format
> strings would have to be double-checked to make sure they are specifying the
> correct thing, so there's a lot of those kind of corrections below.

Thank you for the review Nathan.
I have corrected the patch according to your instructions.

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8651436 [details] [diff] [review]
Remove PR_snprintf calls in xpcom/

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

Thank you!

Comment 6

[Nathan Froyd [:froydnj]]

I've pushed this patch to tryserver to verify that it works correctly:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=335bc4ffc5d9

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/de0ae4f33d74

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/de0ae4f33d74