Incorrect & operator precedence in _PR_UnixSendFile

RESOLVED FIXED

Status

P3
normal
RESOLVED FIXED
19 years ago
19 years ago

People

(Reporter: wtc, Assigned: srinivas)

Tracking

Sun
Solaris

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

19 years ago
In unix.c, _PR_UnixSendFile, we have:
================
    pagesize = PR_GetPageSize();
    /*
     * If the file is large, mmap and send the file in chunks so as
     * to not consume too much virtual address space
     */
    if ((sfd->file_offset == 0) ||
            (sfd->file_offset & (pagesize - 1) == 0)) {
        /*
         * case 1: page-aligned file offset
         */
================

Because the == operator has a higher precedence than
the & operator, the expression (sfd->file_offset & (pagesize - 1) == 0)
gets evaluated as follows:
===>    (sfd->file_offset & ((pagesize - 1) == 0))
===>    (sfd->file_offset & 0)    # assuming (pagesize - 1) is not 0
===>    0

The if statement reduces to:
    if (sfd->file_offset == 0) {

So we fail to detect a nonzero file offset that is page
aligned.  This results in the assertion failure
"addr_offset > 0" at mozilla/nsprpub/pr/src/md/unix.c (rev. 3.29),
line 3005.
(Reporter)

Comment 1

19 years ago
Created attachment 2452 [details] [diff] [review]
A patch file for the socket.c test to cause the assertion failure
(Reporter)

Comment 2

19 years ago
Created attachment 2453 [details] [diff] [review]
Proposed fix.  Just added parentheses around the & operator.
(Reporter)

Updated

19 years ago
Assignee: wtc → srinivas
(Reporter)

Comment 3

19 years ago
The fix is checked in.
/cvsroot/mozilla/nsprpub/pr/src/md/unix/unix.c, revision 3.30.

/m/src/ns/nspr20/pr/src/md/unix/unix.c, revision 2.68

Srinivas, could you look into how to add a new case (where
offset is a multiple of page size) to socket.c?  Thanks.
(Reporter)

Comment 4

19 years ago
This fix is checked into NSPR20_RELEASE_3_5_BRANCH (in the
internal /m/src repository), so it will be in the NSPR
3.5.1 patch release.
/m/src/ns/nspr20/pr/src/md/unix/unix.c, revision 2.65.2.1
(Assignee)

Updated

19 years ago
Status: NEW → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Assignee)

Comment 5

19 years ago
Added a new test case for PR_SendFile in socket.c, for a non-zero page-aligned
file offset.
You need to log in before you can comment on or make changes to this bug.