Closed Bug 132163 Opened 18 years ago Closed 18 years ago
Occurances of uninitialized variables being used before being set (in mailnews/mime)
This bug is just for the "xxx might be used uninitialized" warnings in various source files in mailnews/mime directory. Currently (http://tinderbox.mozilla.org/SeaMonkey/warn1016552220.1515.html - Tue, 19 Mar 2002 10:37 EST) TBox shows the following warnings: mailnews/mime/cthandlers/vcard/nsVCard.cpp:502 `const char * p2' might be used uninitialized in this function mailnews/mime/src/mimecryp.cpp:461 `struct MimeHeaders * outer_headers' might be used uninitialized in this function mailnews/mime/src/mimetext.cpp:433 `nsresult res' might be used uninitialized in this function mailnews/mime/src/mimetext.cpp:434 `int status' might be used uninitialized in this function mailnews/mime/src/mimetpfl.cpp:228 `struct MimeInlineTextPlainFlowedExData * exdata' might be used uninitialized in this function
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1beta
The mimetext.cpp warnings seem to indicate a real problem in the MimeInlineText_open_dam function. First, if text->curDamOffset <= 0 and length <=0, the res will stay uninitialized and in line 452 we will do NS_SUCCEEDED(res) while it is still uninitialized. Second, if both of them (text->curDamOffset and length) are actually 0, then this function would not do anything at all, except free some stuff and set some stuff to nsnull and then return an uninitialized int! Finally, the following code looks very suspicious: 444 if (text->curDamOffset > length) 445 res = MIME_detect_charset(text->lineDamBuffer, text->curDamOffset, &detectedCharset); 446 else 447 res = MIME_detect_charset(text->lineDamBuffer, text->curDamOffset, &detectedCharset); since in both branches of the "if" the code is identical! See http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/mime/src/mimetext.cpp&mark=433,434,444-447,452,489#428
This fixes the first two "might be used uninitialized" warnings as well as fixing couple of other warnings in those files.
Comment on attachment 84374 [details] [diff] [review] Fix some warnings in nsVCard.cpp and mimecryp.cpp Looks good. R=ducarroz
Attachment #84374 - Flags: review+
Comment on attachment 84374 [details] [diff] [review] Fix some warnings in nsVCard.cpp and mimecryp.cpp oops, should use nsnull instead of NULL. No need to continue to use that old stuff.
Comment on attachment 84374 [details] [diff] [review] Fix some warnings in nsVCard.cpp and mimecryp.cpp sr=bienvenu, if you fix the nulls
Attachment #84374 - Flags: superreview+
reply to #2, Both of your points are correct. There is no point to compare "text->curDamOffset" and "length". The if statement should be removed.
NULL -> nsnull
Attachment #84374 - Attachment is obsolete: true
Comment on attachment 84457 [details] [diff] [review] Fix some warnings in nsVCard.cpp and mimecryp.cpp, v2 Carrying forward the sr=bienvenu. Jean-Francois, could you please re-review and commit this? Thanks a lot!
Attachment #84457 - Flags: superreview+
Comment on attachment 84457 [details] [diff] [review] Fix some warnings in nsVCard.cpp and mimecryp.cpp, v2 R=ducarroz. I'll check it in as soon the tree open...
Attachment #84457 - Flags: review+
Shanjian, besides removing the "if" - how should the text->curDamOffset <= 0 and length <=0 cases be handled (e.g. what is the correct return value there)? Thanks!
default status to 0 and res to NS_OK in the time when those variable are declared. Nothing can be wrong if nothing has been done.
Fixing it as suggested by Shanjian.
Comment on attachment 84477 [details] [diff] [review] mimetext.cpp fix r=shanjian
Attachment #84477 - Flags: review+
Patch V2 has been checked in the trunk
Comment on attachment 84477 [details] [diff] [review] mimetext.cpp fix sr=mscott
Attachment #84477 - Flags: superreview+
QA Contact: esther → stephend
Patch V3 has been checked in the trunk
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
According to http://tinderbox.mozilla.org/SeaMonkey/warn1022032500.6496.html, this is fixed. Verified.
Status: RESOLVED → VERIFIED
Well, it's not really fixed - only 4 warnings out of five were silenced with the mimetpfl.cpp warning still there (see the 31-th one under http://tinderbox.mozilla.org/SeaMonkey/warn1022032500.6496.html#rhp ). I guess that's OK - we still have the bug 59673 for warnings in all the mailnews.
This one? Sorry, I missed this somehow. 31.mailnews/mime/src/mimetpfl.cpp:228 (See build log excerpt)`struct MimeInlineTextPlainFlowedExData * exdata' might be used uninitialized in this function 226 // We do it in the beginning so that if an error occur, we can 227 // just free |exdata|. 228 struct MimeInlineTextPlainFlowedExData *exdata; 229 struct MimeInlineTextPlainFlowedExData **prevexdata; 230 prevexdata = &MimeInlineTextPlainFlowedExDataList;
Yes, that one - http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/mime/src/mimetpfl.cpp&mark=228,232#207 . I haven't included it in my other patches since I do not understand why gcc thinks that while ((exdata = *prevexdata) != nsnull) can leave exdata uninitialized. Is it a gcc bug? Or am I missing something?
You need to log in before you can comment on or make changes to this bug.