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

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P3)

Description

[Simon Fraser [no longer active]]

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)

Comment 1

[Simon Fraser [no longer active]]

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.

Comment 2

[Simon Fraser [no longer active]]

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

Comment 3

[Wan-Teh Chang]

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.

Comment 4

[John G. Myers]

How about:

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

Comment 5

[Wan-Teh Chang]

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))

Comment 6

[Wan-Teh Chang]

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

Comment 7

[Wan-Teh Chang]

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

Comment 8

[Wan-Teh Chang]

The fix is checked into the main trunk.
/cvsroot/mozilla/nsprpub/pr/include/prlong.h, revision 3.7

Comment 9

[Wan-Teh Chang]

I added test cases to reproduce the LL_CMP(foo, <=, bar) bug.
/cvsroot/mozilla/nsprpub/pr/tests/lltest.c, revision 3.2.58.1