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.

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

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!

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?

Comment on attachment 109600 [details] [diff] [review] Shut up a few warnings. Moving the review requests.

Comment on attachment 111676 [details] [diff] [review] Shut up a few warnings, v2 Moving the review requests.

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 ;-)

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.

do you need someone to commit this?

> do you need someone to commit this? Yes, please!

done

Thanks! V, all the Bidi warnings went away on brad TBox.