In bug 39011, regarding libSSL's new function ssl_EmulateSendFile, Wan-Teh wrote: > NSPR 4.0 and earlier incorrectly assume that the mem mapping alignment > boundary is the page size. > To work around this NSPR bug, you have to make an OS-dependent call. > When you upgrade to NSPR 4.1 you will be able to call the new function > PR_GetMemMapAlignment() instead. (But you might as well use the new > PR_EmulateSendFile function in NSPR 4.1 directly.) I think NSS is now using NSPR 4.1.x, and so we should consider making the changes Wan-Teh suggested.
Created attachment 204598 [details] [diff] [review] Proposed patch This will eliminate all the code in lib/ssl/emulate.c. See bug 34920 comment 2 on how PR_EmulateAcceptRead and PR_EmulateSendFile should be used, and the transmitfile method should be defined. The only design issue is whether you want to have PR_EmulateAcceptRead and PR_EmulateSendFile directly in the ssl_methods structure, or you want to define trivial ssl_AcceptRead and ssl_SendFile methods that simply call PR_EmulateAcceptRead and PR_EmulateSendFile. Let me know which you prefer. I've verified that PR_EmulateAcceptRead and ssl_EmulateAcceptRead are essentially the same. I still need to compare PR_EmulateSendFile with ssl_EmulateSendFile to make sure they are essentially the same. But please go ahead and review this patch.
Created attachment 204601 [details] [diff] [review] Alternate patch This is the other design I described.
Comment on attachment 204598 [details] [diff] [review] Proposed patch I prefer this patch. r=nelson
Comment on attachment 204601 [details] [diff] [review] Alternate patch This patch is also acceptable, but I think the other patch is preferable.
I checked in the proposed patch on the NSS trunk (3.12). Removing emulate.c; /cvsroot/mozilla/security/nss/lib/ssl/emulate.c,v <-- emulate.c new revision: delete; previous revision: 1.7 done Checking in manifest.mn; /cvsroot/mozilla/security/nss/lib/ssl/manifest.mn,v <-- manifest.mn new revision: 1.14; previous revision: 1.13 done Checking in sslimpl.h; /cvsroot/mozilla/security/nss/lib/ssl/sslimpl.h,v <-- sslimpl.h new revision: 1.44; previous revision: 1.43 done Checking in sslsock.c; /cvsroot/mozilla/security/nss/lib/ssl/sslsock.c,v <-- sslsock.c new revision: 1.46; previous revision: 1.45 done
Comment on attachment 204598 [details] [diff] [review] Proposed patch I would like to eliminate any unnecessary libssl source code differences between the trunk and the 3.11 branch, so I am requesting second review on this patch for the 3.11 branch.
reopening, so this will appear on our radar for 3.11.1
Comment on attachment 204598 [details] [diff] [review] Proposed patch Looks fine. I didn't review the NSPR functions, only the NSS usage.
I back ported this fix to the NSS 3.11 branch.