Closed Bug 126484 Opened 18 years ago Closed 17 years ago

Occurences of uninitialized variables being used before being set (in Bidi files)

Categories

(Core :: Layout: Text and Fonts, defect)

x86
Linux
defect
Not set

Tracking

()

VERIFIED FIXED

People

(Reporter: mozilla-bugs, Assigned: mozilla-bugs)

References

Details

Attachments

(1 file, 1 obsolete file)

This bug is just for the warnings in layout/base/src/nsBidiPresUtils.cpp. Currently
(http://tinderbox.mozilla.org/SeaMonkey/warn1014136200.29882.html - Tue, 19 Feb
2002 11:30 EST) TBox shows the following warnings:

layout/base/src/nsBidiPresUtils.cpp:166
 `nsresult rv' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:222
 `PRInt32 contentOffset' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:228
 `PRBool isTextFrame' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:585
 `nscoord dWidth' might be used uninitialized in this function

P.S. See also bug 126457 that documents similar warnings in the files in the
intl/ directory - most of these warnings are also in *Bidi* files.

P.P.S. See also bug 94151 (now fixed) which documented similar warning in
nsBidiPresUtils.cpp
Bug 59652 is the meta-bug tracking the fight against these (potentially very
nasty) warnings.

P.S. Trying to make sure that 1.0 has as little warnings as possible.
Blocks: 59652
Keywords: mozilla1.0
OK, after some Bidi files were moved around, here are all the "xxx might be used
uninitialized" warnings I currently see in *Bidi*.cpp files:

content/shared/src/nsBidiUtils.cpp:239
 `PRInt8 leftNoTrJ' might be used uninitialized in this function

content/shared/src/nsBidiUtils.cpp:343
 `PRUint32 beginArabic' might be used uninitialized in this function

content/shared/src/nsBidiUtils.cpp:391
 `PRUint32 beginArabic' might be used uninitialized in this function

content/shared/src/nsBidiUtils.cpp:423
 `PRUint32 beginNumeral' might be used uninitialized in this function

layout/base/src/nsBidi.cpp:850
 `DirProp beforeNeutral' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:178
 `nsresult rv' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:234
 `PRInt32 contentOffset' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:240
 `PRBool isTextFrame' might be used uninitialized in this function

layout/base/src/nsBidiPresUtils.cpp:597
 `nscoord dWidth' might be used uninitialized in this function

P.S. Another "dangerous" warning in there:

- content/shared/src/nsBidiUtils.cpp:319 : Statement with no effect
Summary: Occurances of uninitialized variables being used before being set (in nsBidiPresUtils.cpp) → Occurances of uninitialized variables being used before being set (in Bidi files)
Summary: Occurances of uninitialized variables being used before being set (in Bidi files) → Occurences of uninitialized variables being used before being set (in Bidi files)
Attached patch Shut up a few warnings. (obsolete) — Splinter Review
This patch adds initialization for all the variables compiler was complaining
about. It seems that in most cases compiler was just not smart enough, but it's
probably worth fixing anyway (after all, according to bug 179819 such warning
ought to be considered compilation errors).
Comment on attachment 109600 [details] [diff] [review]
Shut up a few warnings.

Please review. Thanks!
Attachment #109600 - Flags: superreview?
Attachment #109600 - Flags: review?
Keywords: mozilla1.0
Attachment #109600 - Flags: superreview?(bzbarsky)
Attachment #109600 - Flags: superreview?
Attachment #109600 - Flags: review?(smontagu)
Attachment #109600 - Flags: review?
Comment on attachment 109600 [details] [diff] [review]
Shut up a few warnings.

>-  PRInt8 leftJ, thisJ, rightJ;
>-  PRInt8 leftNoTrJ, rightNoTrJ;
>-  thisJ = eNJ;
>+  PRInt8 leftJ, thisJ = eNJ, rightJ;
>+  PRInt8 leftNoTrJ = eNJ, rightNoTrJ;

I find this style confusing. Please don't combine multiple declarations and
initializations on the same line, here and later in the patch.
> It seems that in most cases compiler was just not smart enough, but it's
> probably worth fixing anyway

Simon, is any of this code performance-critical to the point where this
statement would be false (I sort of doubt it, but just checking).
Updated not to include several initializations in a variable declaration (per
comment #6). Is this better?
Attachment #109600 - Attachment is obsolete: true
Comment on attachment 109600 [details] [diff] [review]
Shut up a few warnings.

Moving the review requests.
Attachment #109600 - Flags: superreview?(bzbarsky)
Attachment #109600 - Flags: review?(smontagu)
Comment on attachment 111676 [details] [diff] [review]
Shut up a few warnings, v2

Moving the review requests.
Attachment #111676 - Flags: superreview?(bzbarsky)
Attachment #111676 - Flags: review?(smontagu)
Comment on attachment 111676 [details] [diff] [review]
Shut up a few warnings, v2

r=smontagu, with an implied "no" to the question in comment 7 ;-)
Attachment #111676 - Flags: review?(smontagu) → review+
Comment on attachment 111676 [details] [diff] [review]
Shut up a few warnings, v2

>-  PRInt32 firstRun, endRun, limitRun, runCount,
>-  temp, trailingWSStart=mTrailingWSStart;
>+  PRInt32 firstRun, endRun, limitRun, runCount, temp;

Isn't that _removing_ an initialization?
> >-  PRInt32 firstRun, endRun, limitRun, runCount,
> >-  temp, trailingWSStart=mTrailingWSStart;
> >+  PRInt32 firstRun, endRun, limitRun, runCount, temp;
>
> Isn't that _removing_ an initialization?

No, that's removing an unused variable.
Attachment #111676 - Flags: superreview?(bzbarsky) → superreview+
do you need someone to commit this?
Assignee: mkaply → mozilla-bugs
> do you need someone to commit this?

Yes, please!
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Thanks!

V, all the Bidi warnings went away on brad TBox.
Status: RESOLVED → VERIFIED
Duplicate of this bug: 117156
Component: Layout: BiDi Hebrew & Arabic → Layout: Text
QA Contact: zach → layout.fonts-and-text
You need to log in before you can comment on or make changes to this bug.