Last Comment Bug 518504 - Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX, in comm-central
: Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX, in comm-central
Status: VERIFIED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 11.0
Assigned To: Mark Banner (:standard8)
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on:
Blocks: 518502
  Show dependency treegraph
 
Reported: 2009-09-23 21:05 PDT by Serge Gautherie (:sgautherie)
Modified: 2011-12-02 18:24 PST (History)
5 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in comm-central (18.50 KB, patch)
2009-09-25 17:38 PDT, Jae-Seong Lee-Russo
standard8: review+
standard8: superreview+
Details | Diff | Splinter Review
[checked in] The fix v2 (11.71 KB, patch)
2009-11-18 06:29 PST, Mark Banner (:standard8)
standard8: review+
standard8: superreview+
Details | Diff | Splinter Review
patch according to comment 10 (939 bytes, patch)
2011-11-14 13:13 PST, :aceman
standard8: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2009-09-23 21:05:19 PDT
+++ 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 2009-09-25 17:38:14 PDT
Created attachment 402972 [details] [diff] [review]
Replace PR_MIN/PR_MAX with NS_MIN/NS_MAX in comm-central
Comment 2 Serge Gautherie (:sgautherie) 2009-09-25 20:31:26 PDT
(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) 2009-09-26 08:25:40 PDT
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) 2009-11-18 06:19:02 PST
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) 2009-11-18 06:29:19 PST
Created attachment 413065 [details] [diff] [review]
[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) 2009-11-18 09:23:29 PST
(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) 2009-11-18 14:48:46 PST
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 2009-11-18 16:39:48 PST
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) 2009-11-19 01:58:08 PST
(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] 2011-07-06 16:06:51 PDT
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) 2011-07-07 00:59:18 PDT
Sorry, I don't have time to fix this up at the moment.
Comment 12 :aceman 2011-11-14 13:13:47 PST
Created attachment 574391 [details] [diff] [review]
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) 2011-12-02 03:57:07 PST
Checked in: http://hg.mozilla.org/comm-central/rev/e9a00fadaf6c
Comment 14 Serge Gautherie (:sgautherie) 2011-12-02 18:24:34 PST
V.Fixed, per MXR.

Note You need to log in before you can comment on or make changes to this bug.