597249 - Make GlobalPrinters::GetNumPrinters() implementations return a PRUint32 instead of PRInt32

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Daniel Holbert [:dholbert]]

I just noticed that all our (platform-specific) implementations of GlobalPrinters::GetNumPrinters() return PRInt32, when really the "count of printers" is an unsigned value.

Here's the list of implementations:
 http://mxr.mozilla.org/mozilla-central/search?string=%20GetNumPrinters%28

All our implementations are actually backed by unsigned values:
 - for GTK & windows, we return "nsTArray::Length || 0"
 - For OS2, we return a ULONG
 - For BeOS, we return an int which is initialized to 0 and then only ever modified by incrementing (with ++) or being reset to 0.
 - Mac doesn't have this code at all.

So, I'm filing this bug on switching GlobalPrinters::GetNumPrinters() to return PRUint32 instead of PRInt32.

Comment 1

[Felipe Corrêa da Silva Sanches]

Attachment: PRInt32 -> PRUint32

Comment 2

[Daniel Holbert [:dholbert]]

Comment on attachment 476177 [details] [diff] [review]
PRInt32 -> PRUint32

Thanks for the quick patch!

This needs one more thing, though -- to avoid introducing a bunch of unsigned --> signed conversions (and unsigned/signed comparisons), can you change |numPrinters| and |printerInx| from signed to unsigned here:
http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsDeviceContextSpecWin.cpp#921

And the same for numPrinters| & |count| in these two places:
http://mxr.mozilla.org/mozilla-central/source/widget/src/beos/nsDeviceContextSpecB.cpp#154
http://mxr.mozilla.org/mozilla-central/source/widget/src/gtk2/nsDeviceContextSpecG.cpp#719

And I think that should cover it. This needs review from Widget module owner or peer, though.  You should request review from :roc or :vlad once you've done the above. Thanks again!

Comment 3

[Felipe Corrêa da Silva Sanches]

Attachment: PRUint32 usage

Comment 4

[Daniel Holbert [:dholbert]]

Comment on attachment 476239 [details] [diff] [review]
PRUint32 usage

Requesting approval to land this patch -- it just cleans up a bunch of signed integers that really only store unsigned values to be *true* unsigned integers.

Comment 5

[Joe Drew (not getting mail)]

Comment on attachment 476239 [details] [diff] [review]
PRUint32 usage

This can go in post-b7

Comment 6

[Daniel Holbert [:dholbert]]

Pushed: http://hg.mozilla.org/mozilla-central/rev/2ca959dd2275