Closed
Bug 589320
Opened 14 years ago
Closed 10 years ago
Audit printf()s in dom/ipc
Categories
(Core :: IPC, defect)
Core
IPC
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: cjones, Assigned: lfresh)
Details
(Whiteboard: [good first bug])
Attachments
(1 file)
372 bytes,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
We have some hanging around still. They're quickly becoming not-useful and need to be nuked.
Reporter | ||
Updated•11 years ago
|
Whiteboard: [good first bug]
Comment 3•10 years ago
|
||
(In reply to Mike Hoye [:mhoye] from comment #2) > Gaurav - Are you still interested in working on this bug? Yes I am.
Comment 4•10 years ago
|
||
Please go ahead - as soon as you've got a patch ready to go, we can give you ownership of the bug.
Assignee | ||
Comment 5•10 years ago
|
||
I have looked over the code in dom/ipc and do not see an printf()s that seemed inappropriate. In ProcessPriorityManager.cpp it will use printf for logging if ENABLE_LOGGING is defined and that seems as it should be. In ContentProcess.h there is an _MOZ_LOG macro that does not appear to be used anywhere and can probably be removed. diff -r 6a63bcb6e0d3 dom/ipc/ContentProcess.h --- a/dom/ipc/ContentProcess.h Sat Sep 27 03:16:58 2014 -0700 +++ b/dom/ipc/ContentProcess.h Sun Sep 28 17:09:33 2014 -0700 @@ -11,9 +11,6 @@ #include "mozilla/ipc/ScopedXREEmbed.h" #include "ContentChild.h" -#undef _MOZ_LOG -#define _MOZ_LOG(s) printf("[ContentProcess] %s", s) - namespace mozilla { namespace dom {
Comment 6•10 years ago
|
||
Can I work on this bug?
Comment 7•10 years ago
|
||
Hi, Vinay - It looks like Lisa has already got this one in-hand. Can you take a look at http://www.joshmatthews.net/bugsahoy/? and see if there's one there that's relevant to your interests? Thanks.
Assignee | ||
Comment 8•10 years ago
|
||
Not sure who, if anyone can review comment 5. I'm happy to submit a patch but it was such a small change so I didn't.
Flags: needinfo?(mhoye)
Comment 9•10 years ago
|
||
Ok, then throw it up for review. Every little bit helps. Cjones, what do you think?
Flags: needinfo?(mhoye) → needinfo?(cjones.bugs)
Comment 10•10 years ago
|
||
Olli is probably a better person to ask about dom/ipc stuff at this point. Lisa, would you mind attaching a regular patch to the bug? Having a patch around makes the bugzilla process of getting a review etc. easier (so there's something to put an r+ on) etc., even though this patch is very small.
Flags: needinfo?(bugs)
Updated•10 years ago
|
Flags: needinfo?(cjones.bugs)
Updated•10 years ago
|
Flags: needinfo?(bugz42)
Comment 11•10 years ago
|
||
There doesn't seem to be much to do here, as comment 5 hints. There used to be plenty of printfs in dom/ipc, but apparently they have been removed.
Flags: needinfo?(bugs)
Assignee | ||
Comment 12•10 years ago
|
||
Audit found unused printf
Attachment #8503410 -
Flags: review?(cjones.bugs)
Flags: needinfo?(bugz42)
Updated•10 years ago
|
Attachment #8503410 -
Flags: review?(cjones.bugs) → review?(bugs)
Updated•10 years ago
|
Assignee: nobody → bugz42
Comment 13•10 years ago
|
||
Comment on attachment 8503410 [details] [diff] [review] Audit of printf() in dom/ipc Ah yes, that could probably go.
Attachment #8503410 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 14•10 years ago
|
||
Great! Glad to hear it.
Comment 15•10 years ago
|
||
Thanks for looking over things and writing the patch, even though it didn't turn up much. Just for future reference, here's some information about how to make a properly formatted patch: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F (If you use git, let me know and I can find some directions for it.) For this bug, I'll just fix the patch up, assuming you want the same name and email as you use on bugzilla. I'll do some simple local testing and land it.
Assignee | ||
Comment 16•10 years ago
|
||
Oh thanks Andrew! I'm reading this over now so I can format them correctly in the future. I would like to use Mercurial for patches on a Mercurial repo but I do use Git as well. Yes my bugzilla name and email are correct. Thank you.
Comment 17•10 years ago
|
||
I landed the patch on mozilla-inbound. It will automatically be merged into the central repository mozilla-central in the next day or so, and then the bug will be set to FIXED. https://hg.mozilla.org/integration/mozilla-inbound/rev/b3cf4699f538
https://hg.mozilla.org/mozilla-central/rev/b3cf4699f538
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•