556303 - Fix "nsTArray.cpp:88: warning: comparison between signed and unsigned integer expressions"

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Daniel Holbert [:dholbert]]

Just noticed this build warning:
> xpcom/glue/nsTArray.cpp: In member function ‘PRBool nsTArray_base::EnsureCapacity(PRUint32, PRUint32)’:
> xpcom/glue/nsTArray.cpp:88: warning: comparison between signed and unsigned integer expressions

(This warning appears 3 times in the build log given in URL field)

Here's the "guilty" line of code:
> 87   // Use doubling algorithm when forced to increase available capacity.
> 88   capacity = PR_MAX(capacity, mHdr->mCapacity << 1);

Here, capacity and mHdr->mCapacity are both unsigned, **BUT** the latter is declared as being 31 bits wide:
> 140     struct Header {
> 141       PRUint32 mLength;
> 142       PRUint32 mCapacity : 31;

I believe the "<< 1" on line 88 is treated as multiplication by the (signed) integer value "2".  And since mCapacity is a shorter-than-int type, it gets promoted to |int| in order to do this multiplication.  So that gives us |int| * |int| which results in |int|.

So, we need to cast that result back to unsigned in order to avoid the signed-unsigned warning.  Alternately, we could replace "<< 1" by "* 2u" -- that fixes the warning as well.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1: s/<< 1/* 2u/

(In reply to comment #0)
> So, we need to cast that result back to unsigned in order to avoid the
> signed-unsigned warning.

Actually, a PRUint32 cast here would be fragile, because if mCapacity were ever to change its type (to say a 63- or 64-bit value), then the PRUint32 cast could lose information. (if we forgot to change the casted type)

> Alternately, we could replace "<< 1" by "* 2u" --
> that fixes the warning as well.

This solution seems better, in that it doesn't have the fragility problems described above.  The attached patch uses this strategy. (and adds a comment briefly explaining the use of "2u")

Comment 2

[Daniel Holbert [:dholbert]]

Landed: http://hg.mozilla.org/mozilla-central/rev/17e554c5087c