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

Status: RESOLVED FIXED

(Core :: Layout, defect, P1)

Description

[Serge Gautherie (:sgautherie)]

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <jsregexp.c>

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 138621 [details] [diff] [review]
(Av1) <jsregexp.c>


I have no compiler: Could you compile/test/review/check it in ? Thanks.

Comment 3

[Stefan Borggraefe]

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)

Comment 4

[Serge Gautherie (:sgautherie)]

Attachment: (Av1b) <jsregexp.c>

Av1, with comment 3 suggestion(s):
I changed the |type(0)| syntax to |(type)0|...

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 138625 [details] [diff] [review]
(Av1b) <jsregexp.c>


I have no compiler: Could you compile/test/review/check it in ? Thanks.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) <nsPresShell.cpp>

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 138626 [details] [diff] [review]
(Bv1) <nsPresShell.cpp>

I have no compiler: Could you compile/test/review it ? Thanks.

Comment 8

[Serge Gautherie (:sgautherie)]

"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.

Comment 9

[Nicholas Allen]

The js changes are a dupe of bug 59659.

Comment 10

[Serge Gautherie (:sgautherie)]

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 ?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1b) <nsPresShell.cpp> [Checked in: Comment 15]

Bv1, with comment 11 suggestion(s).

Comment 13

[Serge Gautherie (:sgautherie)]

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.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Assigning to myself for checkin

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in the patch I reviewed. What's the status of the other patches?

Comment 16

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) <nsGfxFactoryGTK.cpp> [moved to bug 202594]

Comment 17

[Serge Gautherie (:sgautherie)]

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.

Comment 18

[Serge Gautherie (:sgautherie)]

Comment on attachment 138626 [details] [diff] [review]
(Bv1) <nsPresShell.cpp>

(for the record)
This patch belonged to bug 59675.

Comment 19

[Serge Gautherie (:sgautherie)]

(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 :-<)

Comment 20

[Brendan Eich [:brendan]]

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

Comment 21

[Serge Gautherie (:sgautherie)]

(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 ?