Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Audit printf()s in dom/ipc

RESOLVED FIXED in mozilla36

Status

()

Core
IPC
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: cjones, Assigned: lfresh)

Tracking

unspecified
mozilla36
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment)

We have some hanging around still.  They're quickly becoming not-useful and need to be nuked.
Whiteboard: [good first bug]

Comment 1

4 years ago
Can I work on this?
Flags: needinfo?

Comment 2

3 years ago
Gaurav - Are you still interested in working on this bug?
Flags: needinfo?

Comment 3

3 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

3 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

3 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

3 years ago
Can I work  on this bug?

Comment 7

3 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

3 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

3 years ago
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)
(Assignee)

Comment 12

3 years ago
Created attachment 8503410 [details] [diff] [review]
Audit of printf() in dom/ipc

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+
(Assignee)

Comment 14

3 years ago
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:
  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

3 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.
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
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.