LL_CMP(foo, <=, bar) is broken when !HAVE_LONG_LONG

RESOLVED FIXED

Status

NSPR
NSPR
P3
normal
RESOLVED FIXED
19 years ago
19 years ago

People

(Reporter: Simon Fraser, Assigned: Wan-Teh Chang)

Tracking

All
Mac System 8.5
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

19 years ago
The LL_CMP macro looks like this when HAVE_LONG_LONG is not defined:

#define LL_CMP(a, op, b)    (((PRInt32)(a).hi op (PRInt32)(b).hi) || \
                 (((a).hi == (b).hi) && ((a).lo op (b).lo)))

This fails if |op| is |<=| or |>=|, because the comparison is short-circuited at
the first a.hi |op| b.hi comparison.

This bug required workarounds noted in bug 21189, and will affect a number of
other places in the codebase where LL_CMP is used with <= or >= (e.g. IMAP code)
(Reporter)

Comment 1

19 years ago
cc: brendan, who apparently wrote the macro, and used to have a big comment
saying that it was a no-no to use it with operators that tested for equality.
That comment seems to have gone.
(Reporter)

Comment 2

19 years ago
Note that pr/src/md/unix/unix.c contains an illegal use of the LL_CMP macro (line
2671)
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

19 years ago
How about this?

#define LL_CMP(a, op, b) \
    ((((a).hi != (b).hi) && ((PRInt32)(a).hi op (PRInt32)(b).hi)) || \
    (((a).hi == (b).hi) && ((a).lo op (b).lo)))

This should allow LL_CMP to be used with <, >, <=, and >=.
No need to fix the comments, documentation, and the existing
illegal uses of LL_CMP.

Updated

19 years ago
Depends on: 21473

Comment 4

19 years ago
How about:

#define LL_CMP(a, op, b) \
	(((a).hi == (b).hi) ? ((a).lo op (b).lo) : \
	((PRInt32)(a).hi op (PRInt32)(b).hi))

(Assignee)

Comment 5

19 years ago
John, your solution is right on.  Thanks.

The unsigned version of the macro needs to be fixed
similarly:

#define LL_UCMP(a, op, b) \
        (((a).hi == (b).hi) ? ((a).lo op (b).lo) : \
        ((a).hi op (b).hi))
(Assignee)

Updated

19 years ago
Blocks: 21472
(Assignee)

Comment 6

19 years ago
I checked in the fix for the LL_CMP and LL_UCMP
macros on the NSPRPUB_RELEASE_4_0_BRANCH.
/cvsroot/mozilla/nsprpub/pr/include/prlong.h, revision 3.6.4.1
No longer blocks: 21472
(Assignee)

Comment 7

19 years ago
It turns out that with this change, all the comparison
operators (<, >, <=, >=, ==, and !=) can be used with
the LL_CMP or LL_UCMP macro.
(Assignee)

Comment 8

19 years ago
The fix is checked into the main trunk.
/cvsroot/mozilla/nsprpub/pr/include/prlong.h, revision 3.7
Status: ASSIGNED → RESOLVED
Last Resolved: 19 years ago
Resolution: --- → FIXED
(Assignee)

Updated

19 years ago
Blocks: 21472
(Assignee)

Updated

19 years ago
No longer depends on: 21473
(Assignee)

Updated

19 years ago
Blocks: 21473
(Assignee)

Comment 9

19 years ago
I added test cases to reproduce the LL_CMP(foo, <=, bar) bug.
/cvsroot/mozilla/nsprpub/pr/tests/lltest.c, revision 3.2.58.1
You need to log in before you can comment on or make changes to this bug.