Last Comment Bug 481282 - xpconnect warning fix
: xpconnect warning fix
Status: RESOLVED FIXED
[build_warning]
:
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Atul Aggarwal
:
Mentors:
Depends on:
Blocks: buildwarning
  Show dependency treegraph
 
Reported: 2009-03-03 15:11 PST by Robert Sayre
Modified: 2011-09-05 05:00 PDT (History)
8 users (show)
continuation: in‑testsuite-
continuation: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
silence (1.06 KB, patch)
2009-03-03 16:13 PST, Robert Sayre
mrbkap: review+
mrbkap: superreview+
Details | Diff | Splinter Review
Patch v1 to fix warning (1.16 KB, patch)
2011-09-01 10:26 PDT, Atul Aggarwal
mrbkap: review+
Details | Diff | Splinter Review
Patch v2 (1.84 KB, patch)
2011-09-01 22:43 PDT, Atul Aggarwal
mrbkap: review+
Details | Diff | Splinter Review

Description Robert Sayre 2009-03-03 15:11:52 PST
/Users/sayrer/dev/mozilla-central/js/src/xpconnect/src/nsXPConnect.cpp: 
In member function ‘virtual nsresult nsXPConnect::Traverse(void*, nsCycleCollectionTraversalCallback&)’:

js/src/xpconnect/src/nsXPConnect.cpp:725: warning: ‘clazz’ may be used uninitialized in this function

js/src/xpconnect/src/nsXPConnect.cpp:724: warning: ‘obj’ may be used uninitialized in this function
Comment 1 Robert Sayre 2009-03-03 16:13:19 PST
Created attachment 365323 [details] [diff] [review]
silence
Comment 2 Blake Kaplan (:mrbkap) 2009-03-03 16:52:38 PST
Comment on attachment 365323 [details] [diff] [review]
silence

r+sr=me if you take out the duplicated shadowing definitions many lines lower.
Comment 3 Peter Van der Beken [:peterv] - away till Aug 1st 2009-03-06 06:31:23 PST
Pfff, looks like a bogus warning to me.
Comment 4 Jacek Caban 2010-08-22 04:55:43 PDT
If the patch still needs check-in, it requires approval now.
Comment 5 Atul Aggarwal 2011-09-01 10:26:14 PDT
Created attachment 557558 [details] [diff] [review]
Patch v1 to fix warning
Comment 6 Andrew McCreight [:mccr8] 2011-09-01 10:28:20 PDT
Oh, nice.  This thing has been annoying me for a while.
Comment 7 Blake Kaplan (:mrbkap) 2011-09-01 20:57:11 PDT
Comment on attachment 557558 [details] [diff] [review]
Patch v1 to fix warning

Paraphrasing comment 2:

r=me if you take out the duplicated shadowing definitions many lines lower.
Comment 8 Atul Aggarwal 2011-09-01 21:28:43 PDT
Can you please clarify which lines you are talking about? There are lots of warning in the code and this is the only warning I see in nsXPConnect.cpp file.
Comment 9 Andrew McCreight [:mccr8] 2011-09-01 21:44:27 PDT
I think he means that these lines can be removed:
797             JSObject *obj = static_cast<JSObject*>(p);
798             js::Class *clazz = obj->getClass();

They are assigned identical values on lines 749 and 750, under identical guards as the initial assignment.

That function really needs to be split up or something.
Comment 10 Atul Aggarwal 2011-09-01 22:43:47 PDT
Created attachment 557769 [details] [diff] [review]
Patch v2

Also removing some obsolete assignment as suggested by mrbkap.
Comment 11 Blake Kaplan (:mrbkap) 2011-09-02 00:52:06 PDT
Comment on attachment 557769 [details] [diff] [review]
Patch v2

Thanks!
Comment 12 Andrew McCreight [:mccr8] 2011-09-04 08:12:50 PDT
Do you need somebody to land this for you, Atul?
Comment 13 Atul Aggarwal 2011-09-04 08:30:55 PDT
Andrew, Yes I need some one to land this patch into trunk. Currently, I don't even have access to try server let alone the mozilla-central :)
Comment 14 Andrew McCreight [:mccr8] 2011-09-04 09:01:56 PDT
Okay, I'll land this patch.  In the future, you should set the commit message of patches that you upload.  You can do this like so:

hg qref -m "Bug 481282 - fix uninitialized variable warning in nsXPConnect::Traverse. r=mrbkap"

The message gives the bug number, a description of what the patch does, then the person who reviewed it.  You can see many more examples of commit messages at https://tbpl.mozilla.org/

Not a big deal, but it makes things a little easier for people who commit patches for you.  Thanks again for the patch!
Comment 15 Andrew McCreight [:mccr8] 2011-09-04 09:30:20 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/35f7fb57b504
Comment 16 Atul Aggarwal 2011-09-04 10:47:47 PDT
Thanks a lot Andres. I have already started adding commit messages and all my latest patched do have commit messages in the patch.
Comment 17 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-09-05 05:00:15 PDT
http://hg.mozilla.org/mozilla-central/rev/35f7fb57b504

Note You need to log in before you can comment on or make changes to this bug.