Closed Bug 230397 Opened 16 years ago Closed 16 years ago

In <nsPresShell.cpp>, 2 * ['type var' might be used uninitialized in this function] from 'Blamed Build Warnings; Linux brad Clobber'

Categories

(Core :: Layout, defect, P1, trivial)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8alpha1

People

(Reporter: sgautherie, Assigned: roc)

References

Details

(Whiteboard: [Description: See comment 8])

Attachments

(5 obsolete files)

 
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.7alpha
Summary: Fix all ['type var' might be used uninitialized in this function] from 'Blamed Build Warnings; Linux brad Clobber' → Fix all ['type var' might be used uninitialized in this function] from 'Blamed Build Warnings; Linux brad Clobber'; and code cleanup.
Attached patch (Av1) <jsregexp.c> (obsolete) — Splinter Review
Comment on attachment 138621 [details] [diff] [review]
(Av1) <jsregexp.c>


I have no compiler: Could you compile/test/review/check it in ? Thanks.
Attachment #138621 - Flags: review?(brendan)
This does not compile for me (gcc 3.3.1):

jsregexp.c: In function `parseRegExp':
jsregexp.c:438: error: parse error before "uint16"
jsregexp.c: In function `calculateBitmapSize':
jsregexp.c:699: error: parse error before "jschar"
jsregexp.c:720: error: `c' undeclared (first use in this function)
jsregexp.c:720: error: (Each undeclared identifier is reported only once
jsregexp.c:720: error: for each function it appears in.)
jsregexp.c: In function `processCharSet':
jsregexp.c:1868: error: parse error before "jschar"
jsregexp.c:1903: error: `thisCh' undeclared (first use in this function)
Attached patch (Av1b) <jsregexp.c> (obsolete) — Splinter Review
Av1, with comment 3 suggestion(s):
I changed the |type(0)| syntax to |(type)0|...
Attachment #138621 - Attachment is obsolete: true
Attachment #138621 - Flags: review?(brendan)
Comment on attachment 138625 [details] [diff] [review]
(Av1b) <jsregexp.c>


I have no compiler: Could you compile/test/review/check it in ? Thanks.
Attachment #138625 - Flags: review?(brendan)
Attached patch (Bv1) <nsPresShell.cpp> (obsolete) — Splinter Review
Comment on attachment 138626 [details] [diff] [review]
(Bv1) <nsPresShell.cpp>

I have no compiler: Could you compile/test/review it ? Thanks.
Attachment #138626 - Flags: review?(roc)
"Most" of these warnings are "bogus":
the code is right, but the compiler is "confused" by |if|'s.

Still, it's easier to deal with the "real" warnings - possibly of other kinds -
if the "bogus" ones are not in the way.
Whiteboard: [Description: See comment 8]
The js changes are a dupe of bug 59659.
Blocks: 59652
No longer blocks: buildwarning
Comment on attachment 138625 [details] [diff] [review]
(Av1b) <jsregexp.c>

Re comment 9:

Agreed: see bug 59659 comment 22 (dated 2003-10-23).

brendan:
this patch includes 3 type of changes:
*"dumb" variable initializations
*lexHex/nDigits rewrites
*bad/js_InitRegExpClass() rewrite
Do you care for any of them at this time ?
Whiteboard: [Description: See comment 8] → [Description: See comment 8] [Av1-jsregexp is "dupe" of Bug 59659]
Comment on attachment 138626 [details] [diff] [review]
(Bv1) <nsPresShell.cpp>

I don't like all the changes from == 0 to !. Can you get rid of those? I prefer
to only use ! for booleans and pointers and generally our code does that.
Bv1, with comment 11 suggestion(s).
Attachment #138626 - Attachment description: (Bv1) <nsPreShell.cpp> → (Bv1) <nsPresShell.cpp>
Attachment #138626 - Attachment is obsolete: true
Attachment #138626 - Flags: review?(roc)
Comment on attachment 138728 [details] [diff] [review]
(Bv1b) <nsPresShell.cpp>
[Checked in: Comment 15]


I have no compiler: Could you compile/test/review it ? Thanks.
Attachment #138728 - Flags: review?(roc)
Attachment #138728 - Flags: superreview+
Attachment #138728 - Flags: review?(roc)
Attachment #138728 - Flags: review+
Assigning to myself for checkin
Assignee: gautheri → roc
Status: ASSIGNED → NEW
Priority: -- → P1
Attachment #138625 - Flags: review?(brendan)
checked in the patch I reviewed. What's the status of the other patches?
Attachment #138728 - Attachment description: (Bv1b) <nsPresShell.cpp> → (Bv1b) <nsPresShell.cpp> [Checked in: Comment 15]
Attachment #138728 - Attachment is obsolete: true
Target Milestone: mozilla1.7alpha → mozilla1.8alpha
Comment on attachment 146094 [details] [diff] [review]
(Cv1) <nsGfxFactoryGTK.cpp>
[moved to bug 202594]

I have no compiler: Could you compile/test/(super-)review/check in this patch ?
Thanks.
Attachment #146094 - Flags: superreview?(blizzard)
Attachment #146094 - Flags: review?(blizzard)
Comment on attachment 138626 [details] [diff] [review]
(Bv1) <nsPresShell.cpp>

(for the record)
This patch belonged to bug 59675.
Attachment #146094 - Attachment description: (Cv1) <nsGfxFactoryGTK.cpp> → (Cv1) <nsGfxFactoryGTK.cpp> [moved to bug 202594]
Attachment #146094 - Attachment is obsolete: true
Attachment #146094 - Flags: superreview?(blizzard)
Attachment #146094 - Flags: review?(blizzard)
(In reply to comment #15)
> What's the status of the other patches?

brendan:
You cancelled my request for review on 2004-03-09...
yet, could you answer comment 10 question ?
(I see that you were not in the CC list at that time; fixed :-<)
Serge, I know you don't compile, but can you verify that jsregexp.c has, through
other changes, been fixed to have no such warnings?  I don't see any with gcc
3.3.2 on Fedora Core 1.  I'll leave it to you to obsolete the patch.

/be
Attachment #138625 - Attachment is obsolete: true
No longer depends on: 230001
(In reply to comment #20)
> Serge, I know you don't compile, but can you verify that jsregexp.c has, through
> other changes, been fixed to have no such warnings?  I don't see any with gcc

{ *"dumb" variable initializations }
My mistake: I checked today, but against old v3.75 instead of current v3.79 file :-(
Verified: 'Blamed Build Warnings / Linux brad Clobber' doesn't list any warning
in this file.

> I'll leave it to you to obsolete the patch.

Done.

Still, (for a possible new/other "cleanup/noIssue" bug)

{ *lexHex/nDigits rewrites }
I believe the processing speed could be better without this fix:
do you confirm that it's not wanted ?

{ *bad/js_InitRegExpClass() rewrite }
I think this part could be good to have,
at the very least, getting rid of |bad:|,
don't you ?
Blocks: 59675
No longer blocks: 59652
Status: NEW → RESOLVED
Closed: 16 years ago
Component: Browser-General → Layout: Misc Code
Resolution: --- → FIXED
Summary: Fix all ['type var' might be used uninitialized in this function] from 'Blamed Build Warnings; Linux brad Clobber'; and code cleanup. → In <nsPresShell.cpp>, 2 * ['type var' might be used uninitialized in this function] from 'Blamed Build Warnings; Linux brad Clobber'
Whiteboard: [Description: See comment 8] [Av1-jsregexp is "dupe" of Bug 59659] → [Description: See comment 8]
Product: Core → Core Graveyard
Component: Layout: Misc Code → Layout
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.