518504 - Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX, in comm-central

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Serge Gautherie (:sgautherie)]

+++ This bug was initially created as a clone of Bug #512106 +++

See that bug about why and how to do it...

Comment 1

[Jae-Seong Lee-Russo]

Attachment: Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in comm-central

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> Created an attachment (id=402972) [details]
> Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in comm-central

I pushed it to TryServer...

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 402972 [details] [diff] [review]
Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in comm-central


(In reply to comment #2)
> I pushed it to TryServer...

Passed as '518504-comm-central__again'.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 402972 [details] [diff] [review]
Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in comm-central

Sorry for the delay in getting to this, I've been very busy with Thunderbird 3 specifics the last couple of months.

>-  PRUint32 maxHdrs= PR_MIN(nsMsgSearchAttrib::OtherHeader + numHeaders+1, nsMsgSearchAttrib::kNumMsgSearchAttributes);
>+  PRUint32 maxHdrs= NS_MIN(nsMsgSearchAttrib::OtherHeader + numHeaders+1, (PRUint32)nsMsgSearchAttrib::kNumMsgSearchAttributes);

nit: I know this was just like it was, but please insert a space before the equals, and a space either side of the + in the numHeaders+1 bit.

>-        rv =  aOutStream->WriteFrom(mInStream, PR_MIN(avail, 4096), &bytesWritten);
>+        rv =  aOutStream->WriteFrom(mInStream, NS_MIN(avail, 4096U), &bytesWritten);

nit: please drop the second space after equals.

>diff --git a/mailnews/imap/src/nsImapMailFolder.cpp b/mailnews/imap/src/nsImapMailFolder.cpp

>-            rv = outputStream->Write(inputBuffer, PR_MIN((PRInt32) bytesRead, bytesLeft), &bytesWritten);
>-            NS_ASSERTION((PRInt32) bytesWritten == PR_MIN((PRInt32) bytesRead, bytesLeft), "wrote out incorrect number of bytes");
>+            rv = outputStream->Write(inputBuffer, NS_MIN((PRInt32) bytesRead, bytesLeft), &bytesWritten);
>+            NS_ASSERTION((PRInt32) bytesWritten == NS_MIN((PRInt32) bytesRead, bytesLeft), "wrote out incorrect number of bytes");

This bitrotted but I fixed it locally.

r/sr=Standard8

Thanks for doing the patch. Normally I'd get you to address the nits, but as I feel rotten for letting this hang for so long, I'll check it in in a few minutes.

Comment 5

[Mark Banner (:standard8)]

Attachment: [checked in] The fix v2

The patch with my comments address.

Checked in:
http://hg.mozilla.org/comm-central/rev/fb7d28871960

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to comment #5)
> http://hg.mozilla.org/comm-central/rev/fb7d28871960

{
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1258555755.1258556056.751.gz
Linux x86-64 comm-central-trunk build on 2009/11/18 06:49:15
	
/builds/slave/comm-central-trunk-linux64/build/mailnews/base/src/nsMsgBiffManager.cpp: In member function ‘nsresult nsMsgBiffManager::SetNextBiffTime(nsBiffEntry&, nsTime)’:
/builds/slave/comm-central-trunk-linux64/build/mailnews/base/src/nsMsgBiffManager.cpp:256: error: no matching function for call to ‘NS_MIN(PRInt64&, long long int)’
}

Comment 7

[Mark Banner (:standard8)]

I've backed out the change in just that file:

http://hg.mozilla.org/comm-central/rev/56e7f2b3ab98

I think I know how to fix this properly but want to investigate a bit more first.

Comment 8

[Takanori MATSUURA]

Still error in
comm-central/mailnews/imap/src/nsImapUrl.cpp: In member function 'virtual nsresult nsImapUrl::GetListOfMessageIds(nsACString_internal&)':
comm-central/mailnews/imap/src/nsImapUrl.cpp:308: error: no matching function for call to 'NS_MIN(PRInt32&, long int)'

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> Still error in
> comm-central/mailnews/imap/src/nsImapUrl.cpp: In member function 'virtual
> nsresult nsImapUrl::GetListOfMessageIds(nsACString_internal&)':
> comm-central/mailnews/imap/src/nsImapUrl.cpp:308: error: no matching function
> for call to 'NS_MIN(PRInt32&, long int)'

Checked in a bustage fix for this bit:
http://hg.mozilla.org/comm-central/rev/3d346eb49f64

Still need to fix up the other bit I backed out.

Comment 10

[Ed Morley [:emorley]]

http://mxr.mozilla.org/comm-central/search?string=PR_M%28IN%7CAX%29%5C%28&regexp=on&case=on&find=%2Fmailnews%2F.*%5C.cpp%24

...shows just one instance left in comm-central:
http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgBiffManager.cpp#294
(the part backed out in comment 6 / comment 7.

Presume something like
> jitter = NS_MAX<PRInt64>(1000000LL, NS_MIN<PRInt64>(jitter, 30000000LL));
...would fix the build error.

Don't have a comm-central repo checked out, so can't sort myself.

Once this bug is closed, all of comm-central and also mozilla-central are PR_M(IN|AX) free :-)

Comment 11

[Mark Banner (:standard8)]

Sorry, I don't have time to fix this up at the moment.

Comment 12

[:aceman]

Attachment: patch according to comment 10

I didn't get a build problem even with the original version (on linux 32bit, gcc 4.5), but here is the update according to comment 10. Maybe it is needed on 64bit.

Comment 13

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/e9a00fadaf6c

Comment 14

[Serge Gautherie (:sgautherie)]

V.Fixed, per MXR.