Closed Bug 745659 Opened 12 years ago Closed 12 years ago

Remove obsolete nsPrintfCString constructor which takes a length

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla15

People

(Reporter: justin.lebar+bug, Assigned: showdown345)

References

Details

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

Attachments

(1 file, 2 obsolete files)

We should just remove this constructor and all the code which calls it.
The changes were able to compile. No new tests written because I only removed code.
Attachment #617332 - Flags: review?(justin.lebar+bug)
\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?)
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-
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.
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)
The hg headers in the new diff look good; thanks.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=ce5c67c21fec
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+
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
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)
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
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.