xpconnect warning fix

RESOLVED FIXED in mozilla9

Status

()

Core
XPConnect
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: Robert Sayre, Assigned: Atul Aggarwal)

Tracking

(Blocks: 1 bug)

Trunk
mozilla9
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
/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
(Reporter)

Comment 1

8 years ago
Created attachment 365323 [details] [diff] [review]
silence
(Reporter)

Updated

8 years ago
Attachment #365323 - Flags: superreview?(mrbkap)
Attachment #365323 - Flags: review?(mrbkap)

Updated

8 years ago
Attachment #365323 - Attachment is patch: true
Attachment #365323 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 365323 [details] [diff] [review]
silence

r+sr=me if you take out the duplicated shadowing definitions many lines lower.
Attachment #365323 - Flags: superreview?(mrbkap)
Attachment #365323 - Flags: superreview+
Attachment #365323 - Flags: review?(mrbkap)
Attachment #365323 - Flags: review+
Pfff, looks like a bogus warning to me.

Updated

7 years ago
Keywords: checkin-needed
OS: Mac OS X → Windows 7

Updated

7 years ago
Assignee: nobody → sayrer

Comment 4

7 years ago
If the patch still needs check-in, it requires approval now.

Updated

7 years ago
Keywords: checkin-needed

Updated

6 years ago
Whiteboard: [build_warning]

Updated

6 years ago
Blocks: 187528
(Assignee)

Comment 5

6 years ago
Created attachment 557558 [details] [diff] [review]
Patch v1 to fix warning
Assignee: sayrer → atulagrwl
Attachment #365323 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #557558 - Flags: review?(mrbkap)
Oh, nice.  This thing has been annoying me for a while.
OS: Windows 7 → All
Hardware: x86 → All
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.
Attachment #557558 - Flags: review?(mrbkap) → review+
(Assignee)

Comment 8

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

Comment 10

6 years ago
Created attachment 557769 [details] [diff] [review]
Patch v2

Also removing some obsolete assignment as suggested by mrbkap.
Attachment #557558 - Attachment is obsolete: true
Attachment #557769 - Flags: review?(mrbkap)
Comment on attachment 557769 [details] [diff] [review]
Patch v2

Thanks!
Attachment #557769 - Flags: review?(mrbkap) → review+

Updated

6 years ago
Attachment #557769 - Attachment is patch: true
Do you need somebody to land this for you, Atul?
Version: unspecified → Trunk
(Assignee)

Comment 13

6 years ago
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 :)
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!
http://hg.mozilla.org/integration/mozilla-inbound/rev/35f7fb57b504
Flags: in-testsuite-
Flags: in-litmus-
Target Milestone: --- → mozilla9
(Assignee)

Comment 16

6 years ago
Thanks a lot Andres. I have already started adding commit messages and all my latest patched do have commit messages in the patch.
http://hg.mozilla.org/mozilla-central/rev/35f7fb57b504
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.