Closed Bug 589320 Opened 14 years ago Closed 10 years ago

Audit printf()s in dom/ipc


(Core :: IPC, defect)

Not set





(Reporter: cjones, Assigned: lfresh)


(Whiteboard: [good first bug])


(1 file)

We have some hanging around still.  They're quickly becoming not-useful and need to be nuked.
Whiteboard: [good first bug]
Can I work on this?
Flags: needinfo?
Gaurav - Are you still interested in working on this bug?
Flags: needinfo?
(In reply to Mike Hoye [:mhoye] from comment #2)
> Gaurav - Are you still interested in working on this bug?

Yes I am.
Please go ahead - as soon as you've got a patch ready to go, we can give you ownership of the bug.
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 {
Can I work  on this bug?
Hi, Vinay - 

It looks like Lisa has already got this one in-hand. Can you take a look at

and see if there's one there that's relevant to your interests?

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)
Ok, then throw it up for review. Every little bit helps. Cjones, what do you think?
Flags: needinfo?(mhoye) → needinfo?(cjones.bugs)
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)
Flags: needinfo?(cjones.bugs)
Flags: needinfo?(bugz42)
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)
Audit found unused printf
Attachment #8503410 - Flags: review?(cjones.bugs)
Flags: needinfo?(bugz42)
Attachment #8503410 - Flags: review?(cjones.bugs) → review?(bugs)
Assignee: nobody → bugz42
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+
Great! Glad to hear it.
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:

(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.
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.
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.
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.