Closed
Bug 208627
Opened 22 years ago
Closed 22 years ago
Incorrect comparison in nsMailDatabase.cpp
Categories
(MailNews Core :: Database, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: tenthumbs, Assigned: Bienvenu)
Details
Attachments
(1 file)
683 bytes,
patch
|
sspitzer
:
review+
sspitzer
:
superreview+
|
Details | Diff | Splinter Review |
Gcc says:
nsMsgDBView.cpp:5204: warning: \
comparison of unsigned expression >= 0 is always true
541 *aResult = PR_ABS(actualFolderTimeStamp - folderDate) <=
gTimeStampLeeway;
which doesn't work because folderDate is unsigned. You probably want something like
aResult = (folderDate - gTimeStampLeeway <= actualFolderTimeStamp)
&& (actualFolderTimeStamp <= folderDate + gTimeStampLeeway);
Assignee | ||
Comment 1•22 years ago
|
||
actually, it does work, at least on windows, because the code does the right
thing. Both numbers are PRUint32's. Also, this code isn't in nsMsgDBView.cpp,
it's in nsMsgDatabase.cpp
I think perhaps the compiler is really complaining about this line:
PRBool nsMsgDBView::IsValidIndex(nsMsgViewIndex index)
{
return ((index >=0) && (index < (nsMsgViewIndex) m_keys.GetSize()));
which makes sense because index really is unsigned. Maybe there are two warnings
and you've combined them somehow...
Yeah, nsMsgDBView.cpp is the warning just above the one I wanted. I
can't copy and paste. :-) Sorry about that. The file in the summary is
the correct one as is the extracted code.
nsMailDatabase.cpp:541: warning:
comparison of unsigned expression < 0 is always false
Gcc is much stricter than most Windows compilers about unsigned
comparisons and it prefers to evaluate expressiosn at compile time
whenever it can.
PR_ABS(a) is something like
((a < 0) ? -a : a)
so the gcc warning means gcc knows a is always positive so it knows the
-a is unreachable so it eliminates it and gcc compiles this as
(a)
which is not what you want. If actualFolderTimeStamp > folderDate then
aResult = 0 always (well, for the most likely cases).
Let me make it clear, if either p or q are unsigned and p = q - 1,
then gcc will evaluate p - q as UNINT_MAX not -1 so the code as it is
doesn't work.
Assignee | ||
Comment 4•22 years ago
|
||
this cast fixes the warning.
Comment 5•22 years ago
|
||
Comment on attachment 125140 [details] [diff] [review]
proposed fix
r/sr=sspitzer
Attachment #125140 -
Flags: superreview+
Attachment #125140 -
Flags: review+
Assignee | ||
Comment 6•22 years ago
|
||
fix checked in.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: MailNews → Core
Updated•17 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•