PRBool success might be used uninitialized ... (nsBulletFrame line 1114)

RESOLVED DUPLICATE of bug 534171

Status

()

Core
Layout
RESOLVED DUPLICATE of bug 534171
13 years ago
6 years ago

People

(Reporter: Ben Fowler, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050418 Firefox/1.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.7) Gecko/20050418 Firefox/1.0.3

At and below  
http://lxr.mozilla.org/mozilla/source/layout/generic/nsBulletFrame.cpp#1114
if any of the first four switch branches are taken, then indeed 'success'
might be used without being assigned to.

Maybe this can't happen.

There may be more than one way of fixing this warning - would anyone like a 
patch?

Reproducible: Always
Summary: PRBool success might be used uninitalized ... (line 1114) → PRBool success might be used uninitalized ... (nsBulletFrame line 1114)

Comment 1

13 years ago
Confirming, if you are willing to make a patch please do.

I would suggest pre-setting success to PR_True rather than mucking with those
first 4 case's.  Though if you have another idea, go for it. [Please note I am
not a reviewer]
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

13 years ago
Created attachment 183580 [details] [diff] [review]
Patch to fix compiler warning, by ensuring that success is always initialised

Add initalisation of sucess to PR_TRUE, and some comments. PR_TRUE
(requested conversion succeeded) is the default condition, PR_FALSE
implies that some fall-back operation took place.
Attachment #183580 - Flags: review?

Comment 3

13 years ago
I would suggest requesting review from a particular person, (preferrably one
with a shorter review que)

Use
http://www.mozilla.org/owners.html  if you are unsure.
(Reporter)

Comment 4

13 years ago
Thank you. It might be David Baron, but I don't know enough about requesting 
review and review queues to jump in!
generally documentation for functions is put in the header file...

Comment 6

13 years ago
Comment on attachment 183580 [details] [diff] [review]
Patch to fix compiler warning, by ensuring that success is always initialised

Targetting review and adding sr? since this should be a simple fix; Though I am
with biesi on that the documentation section should be in the class
declaration, not in its implimentation.
Attachment #183580 - Flags: superreview?(dbaron)
Attachment #183580 - Flags: review?(dbaron)
Attachment #183580 - Flags: review?
Comment on attachment 183580 [details] [diff] [review]
Patch to fix compiler warning, by ensuring that success is always initialised

The massive comment at the beginning of AppendCounterText is way too verbose,
and probably belongs in the header file anyway.  Other than the @param lines,
most of it is either unnecessary or incorrect.

The comment about retrying is also incorrect.  It's not clear why the boolean
return exists at all, but it's not for retrying, since the function can return
false even when it does append text to the string.
Attachment #183580 - Flags: superreview?(dbaron)
Attachment #183580 - Flags: superreview-
Attachment #183580 - Flags: review?(dbaron)
Attachment #183580 - Flags: review-
(Reporter)

Comment 8

13 years ago
Created attachment 183893 [details] [diff] [review]
Improved patch

Shortened comment and removed comment, as above
Attachment #183580 - Attachment is obsolete: true
Attachment #183893 - Flags: review?
Ben, you want to request review from a specific person (see
http://www.mozilla.org/owners.html for the relevant module owners), not review
in general...
(Reporter)

Comment 10

13 years ago
Comment on attachment 183893 [details] [diff] [review]
Improved patch

Requesting review from dbaron. Sorry for the hold-up
Attachment #183893 - Flags: review? → review?(dbaron)
Summary: PRBool success might be used uninitalized ... (nsBulletFrame line 1114) → PRBool success might be used uninitialized ... (nsBulletFrame line 1114)
Comment on attachment 183893 [details] [diff] [review]
Improved patch

+ * @param result The nsString to be appended to
+  **/

wrong indentation on this second line, and is there a reason you did not put
the comment in the header file?
(Reporter)

Comment 12

13 years ago
Created attachment 186843 [details] [diff] [review]
Removed comment

White space change was probably an editing error. I have removed the
comment altogether - it is obviously out of place
Attachment #183893 - Attachment is obsolete: true
Attachment #186843 - Flags: review?
(Reporter)

Updated

13 years ago
Attachment #186843 - Flags: review? → review?(dbaron)
This patch got review but got never checked in.
Do we need an updated patch or is this now obsolete since the PRBool-> Bool switch ?
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 534171
You need to log in before you can comment on or make changes to this bug.