208627 - Incorrect comparison in nsMailDatabase.cpp

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[tenthumbs]

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

Comment 1

[David :Bienvenu]

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

Comment 2

[tenthumbs]

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



 
  

Comment 3

[tenthumbs]

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.

Comment 4

[David :Bienvenu]

Attachment: proposed fix

this cast fixes the warning.

Comment 5

[(not reading, please use [e-mail] instead)]

Comment on attachment 125140 [details] [diff] [review]
proposed fix

r/sr=sspitzer

Comment 6

[David :Bienvenu]

fix checked in.