Remove obsolete nsPrintfCString constructor which takes a length

RESOLVED FIXED in mozilla15

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Justin Lebar (not reading bugmail), Assigned: Patrick)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=jlebar][lang=c++])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
We should just remove this constructor and all the code which calls it.
(Assignee)

Comment 1

5 years ago
Created attachment 617332 [details] [diff] [review]
Removes the nsPrintfCString constructor which takes a length and all subsequent calls

The changes were able to compile. No new tests written because I only removed code.
Attachment #617332 - Flags: review?(justin.lebar+bug)
(Reporter)

Comment 2

5 years ago
\o/

This looks good.  I'm going to run it through tryserver just to make sure we're not missing something on a platform you didn't test on.  The live results are on tbpl [1], and the bot should post a comment in this bug when the tests complete.

These first bugs are also about figuring out mechanics, so I should mention: This is kind of a weird looking patch.  It looks like maybe you concatenated the result of a few hg export's (perhaps hg did that for you?).  We usually look for a patch which has just one "hg export" header.

In that header, you'll want to make the commit message take the form

  "Bug 745659 - What was done to fix the bug. r=jlebar"

Once this patch passes the tests and you upload a patch with the right commit header, I'll check it in for you!

(bsmedberg, I assume you're OK with me reviewing this?)
(Reporter)

Comment 3

5 years ago
er, and the tbpl link:

[1] https://tbpl.mozilla.org/?tree=Try&rev=e3028a232847
(Reporter)

Comment 4

5 years ago
Comment on attachment 617332 [details] [diff] [review]
Removes the nsPrintfCString constructor which takes a length and all subsequent calls

r- because this is burning Linux and MacOS on try. 

If you want to be able to push directly to try, we can get that set up for you.  You need L1 access, and I can vouch for you.

http://www.mozilla.org/hacking/commit-access-policy/
Attachment #617332 - Flags: review?(justin.lebar+bug) → review-

Comment 5

5 years ago
Try run for e3028a232847 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=e3028a232847
Results (out of 119 total builds):
    success: 91
    warnings: 19
    failure: 9
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-e3028a232847
Assignee: nobody → showdown345
jlebar, yes please review away, I went ahead and added you as an official peer of the String module also.
(Assignee)

Comment 7

5 years ago
Created attachment 617651 [details] [diff] [review]
New patch that fixes the errors reported by the build failure report

I looked at the build failure report and was able to locate and fix those errors. I cleaned the build directory and everything recompiles without error. Attached is the new patch. Sorry about the inconvenience.
Attachment #617332 - Attachment is obsolete: true
Attachment #617651 - Flags: review?(justin.lebar+bug)
(Reporter)

Comment 8

5 years ago
The hg headers in the new diff look good; thanks.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=ce5c67c21fec
(Reporter)

Comment 9

5 years ago
Comment on attachment 617651 [details] [diff] [review]
New patch that fixes the errors reported by the build failure report

>diff -r df1bdf389d39 -r 6514ffcd91f5 netwerk/ipc/NeckoCommon.h
>--- a/netwerk/ipc/NeckoCommon.h	Sun Apr 22 12:07:59 2012 -0400
>+++ b/netwerk/ipc/NeckoCommon.h	Sun Apr 22 12:14:36 2012 -0400
>@@ -74,7 +74,7 @@
> 
> #define DROP_DEAD()                                                            \
>   do {                                                                         \
>-    nsPrintfCString msg(1000,"NECKO ERROR: '%s' UNIMPLEMENTED",                \
>+    nsPrintfCString msg("NECKO ERROR: '%s' UNIMPLEMENTED",                \
>                         __FUNCTION__);                                         \
>     NECKO_MAYBE_ABORT(msg);                                                    \
>     return NS_ERROR_NOT_IMPLEMENTED;                                           \
>@@ -82,7 +82,7 @@
> 
> #define ENSURE_CALLED_BEFORE_ASYNC_OPEN()                                      \
>   if (mIsPending || mWasOpened) {                                              \
>-    nsPrintfCString msg(1000, "'%s' called after AsyncOpen: %s +%d",           \
>+    nsPrintfCString msg("'%s' called after AsyncOpen: %s +%d",           \
>                         __FUNCTION__, __FILE__, __LINE__);                     \
>     NECKO_MAYBE_ABORT(msg);                                                    \
>   }                                                                            \

Would you mind if I was really anal and asked you to fix this whitespace?

Also, in the future, please attach -U8 -p diffs.  I have in my hgrc:

  [diff]
  git = 1
  showfunc = 1
  unified = 8

Looks good aside from that, and looks good on try.  If you fix the whitespace, I'll check this in.
Attachment #617651 - Flags: review?(justin.lebar+bug) → review+

Comment 10

5 years ago
Try run for ce5c67c21fec is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=ce5c67c21fec
Results (out of 218 total builds):
    exception: 1
    success: 181
    warnings: 36
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jlebar@mozilla.com-ce5c67c21fec
(Assignee)

Comment 11

5 years ago
Created attachment 618003 [details] [diff] [review]
Bug-745659 - Patch 3

Removes the nsPrintfCString constructor which takes a length and all subsequent calls. It also fixes the extra spaces issue.
Attachment #617651 - Attachment is obsolete: true
Attachment #618003 - Flags: review?(justin.lebar+bug)
(Reporter)

Comment 12

5 years ago
Comment on attachment 618003 [details] [diff] [review]
Bug-745659 - Patch 3

Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/da03b8dcabe8

Congratulations on your first patch!
Attachment #618003 - Flags: review?(justin.lebar+bug) → review+
https://hg.mozilla.org/mozilla-central/rev/da03b8dcabe8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.