fix size_t/int warnings

fix size_t/int warnings

BUG=skia:

Committed: http://code.google.com/p/skia/source/detail?r=14332

[mtklein, 2014-04-23 13:36:17]

What warning's triggering, on which build?  Once we get these fixed up I'd like
to put it everywhere.

https://codereview.chromium.org/247753003/diff/1/include/core/SkEndian.h
File include/core/SkEndian.h (right):

https://codereview.chromium.org/247753003/diff/1/include/core/SkEndian.h#newc...
include/core/SkEndian.h:32: static inline uint16_t SkEndianSwap16(size_t value)
{
If you feel like being a little fancier, I think

template <typename T>
static inline uint16_t SkEndianSwap16(T value) {
   ...
}

conveys the we-take-anything nature of the function.

https://codereview.chromium.org/247753003/diff/1/include/core/SkEndian.h#newc...
include/core/SkEndian.h:55: static inline uint32_t SkEndianSwap32(size_t value)
{
This guy seems like he really wants to be u32 -> u32.  Maybe the fix is doing
the SkToU32 outside the call?

https://codereview.chromium.org/247753003/diff/1/include/core/SkReader32.h
File include/core/SkReader32.h (right):

https://codereview.chromium.org/247753003/diff/1/include/core/SkReader32.h#ne...
include/core/SkReader32.h:36: uint32_t size() const { return SkToU32(fStop -
fBase); }
Perhaps promote these guys to size_t too?

https://codereview.chromium.org/247753003/diff/1/src/core/SkRegion.cpp
File src/core/SkRegion.cpp (right):

https://codereview.chromium.org/247753003/diff/1/src/core/SkRegion.cpp#newcod...
src/core/SkRegion.cpp:799: start[-1] = SkToS32(len >> 1);
What's going on here?  SkRegion::RunType is an int?  We're punning?

https://codereview.chromium.org/247753003/diff/1/src/core/SkScalerContext.cpp
File src/core/SkScalerContext.cpp (right):

https://codereview.chromium.org/247753003/diff/1/src/core/SkScalerContext.cpp...
src/core/SkScalerContext.cpp:551: SkASSERT(srcRB >= (size_t)width);
Is it sensible to add in SkASSERT(width >= 0) here?  It'd make me stop worrying
about conversions and signs.

https://codereview.chromium.org/247753003/diff/1/src/core/SkStream.cpp
File src/core/SkStream.cpp (right):

https://codereview.chromium.org/247753003/diff/1/src/core/SkStream.cpp#newcod...
src/core/SkStream.cpp:152: bool SkWStream::writePackedUInt(size_t value) {
Can we switch this to take a uint32_t instead?  Seems we're fibbing a bit that
we can write any size_t.

[reed1, 2014-04-23 14:04:21]

https://codereview.chromium.org/247753003/diff/1/include/core/SkEndian.h
File include/core/SkEndian.h (right):

https://codereview.chromium.org/247753003/diff/1/include/core/SkEndian.h#newc...
include/core/SkEndian.h:32: static inline uint16_t SkEndianSwap16(size_t value)
{
On 2014/04/23 13:36:18, mtklein wrote:
> If you feel like being a little fancier, I think
> 
> template <typename T>
> static inline uint16_t SkEndianSwap16(T value) {
>    ...
> }
> 
> conveys the we-take-anything nature of the function.

don't want to take any non-integral values (e.g. ptrs, floats, etc). Is size_t
sufficient, or would uintptr_t be cleaner?

https://codereview.chromium.org/247753003/diff/1/include/core/SkEndian.h#newc...
include/core/SkEndian.h:55: static inline uint32_t SkEndianSwap32(size_t value)
{
On 2014/04/23 13:36:18, mtklein wrote:
> This guy seems like he really wants to be u32 -> u32.  Maybe the fix is doing
> the SkToU32 outside the call?

well, so does swap16, but the caller may give us larger types

https://codereview.chromium.org/247753003/diff/1/include/core/SkReader32.h
File include/core/SkReader32.h (right):

https://codereview.chromium.org/247753003/diff/1/include/core/SkReader32.h#ne...
include/core/SkReader32.h:36: uint32_t size() const { return SkToU32(fStop -
fBase); }
On 2014/04/23 13:36:18, mtklein wrote:
> Perhaps promote these guys to size_t too?

Done.

https://codereview.chromium.org/247753003/diff/1/src/core/SkRegion.cpp
File src/core/SkRegion.cpp (right):

https://codereview.chromium.org/247753003/diff/1/src/core/SkRegion.cpp#newcod...
src/core/SkRegion.cpp:799: start[-1] = SkToS32(len >> 1);
On 2014/04/23 13:36:18, mtklein wrote:
> What's going on here?  SkRegion::RunType is an int?  We're punning?

It is an int32_t

https://codereview.chromium.org/247753003/diff/1/src/core/SkScalerContext.cpp
File src/core/SkScalerContext.cpp (right):

https://codereview.chromium.org/247753003/diff/1/src/core/SkScalerContext.cpp...
src/core/SkScalerContext.cpp:551: SkASSERT(srcRB >= (size_t)width);
On 2014/04/23 13:36:18, mtklein wrote:
> Is it sensible to add in SkASSERT(width >= 0) here?  It'd make me stop
worrying
> about conversions and signs.

yes, width should be >= 0. Will add that assert.

https://codereview.chromium.org/247753003/diff/1/src/core/SkStream.cpp
File src/core/SkStream.cpp (right):

https://codereview.chromium.org/247753003/diff/1/src/core/SkStream.cpp#newcod...
src/core/SkStream.cpp:152: bool SkWStream::writePackedUInt(size_t value) {
On 2014/04/23 13:36:18, mtklein wrote:
> Can we switch this to take a uint32_t instead?  Seems we're fibbing a bit that
> we can write any size_t.

I think the better change (but perhaps not in this CL) is to actually store
8-bytes when needed.

[reed1, 2014-04-23 14:06:00]

actually, maybe you're right and we should force the call-sites to cast down to
32 or 16 for the swap calls...

[bungeman-skia, 2014-04-23 14:15:12]

My 2 cents.

https://codereview.chromium.org/247753003/diff/20001/include/core/SkEndian.h
File include/core/SkEndian.h (right):

https://codereview.chromium.org/247753003/diff/20001/include/core/SkEndian.h#...
include/core/SkEndian.h:32: static inline uint16_t SkEndianSwap16(size_t value)
{
Why size_t? It makes no sense. Maybe you want uintmax_t? Note that you probably
don't want uintptr_t, I believe all of these operations (shifts and casts) are
technically undefined behavior on uintptr_t, and it might be the wrong size
anyway.

https://codereview.chromium.org/247753003/diff/40001/include/core/SkEndian.h
File include/core/SkEndian.h (right):

https://codereview.chromium.org/247753003/diff/40001/include/core/SkEndian.h#...
include/core/SkEndian.h:54: static inline uint32_t SkEndianSwap32(uint32_t v) {
Any reason to change this from 'value'?

[reed1, 2014-04-23 14:27:36]

ptal

https://codereview.chromium.org/247753003/diff/40001/include/core/SkEndian.h
File include/core/SkEndian.h (right):

https://codereview.chromium.org/247753003/diff/40001/include/core/SkEndian.h#...
include/core/SkEndian.h:54: static inline uint32_t SkEndianSwap32(uint32_t v) {
On 2014/04/23 14:15:13, bungeman1 wrote:
> Any reason to change this from 'value'?

historical from previous patches. will revert.

[mtklein, 2014-04-23 14:36:28]

lgtm

[reed1, 2014-04-23 14:42:02]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-04-23 14:42:18]

CQ is trying da patch. Follow status at
https://skia-tree-status.appspot.com/cq/reed@google.com/247753003/60001

[commit-bot: I haz the power, 2014-04-23 19:19:58]

Change committed as 14332