Closed Bug 556303 Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: XPCOM, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.3a4

People

(Reporter: dholbert, Assigned: dholbert)

References

()

Details

(Whiteboard: [build_warning])

Attachments

(1 file)

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.
Blocks: 411061
(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")
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #436217 - Flags: review?(benjamin)
Attachment #436217 - Flags: review?(benjamin) → review+
Landed: http://hg.mozilla.org/mozilla-central/rev/17e554c5087c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a4
Whiteboard: [build_warning]
You need to log in before you can comment on or make changes to this bug.