Change Read size to a size_t for clarity

Change Read() size and return value to a size_t for clarity







BUG=100641
TEST=N/A
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=106533

[Ami GONE FROM CHROMIUM, 2011-10-19 21:14:58]

nothin' but nits.

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.cc
File media/filters/ffmpeg_demuxer.cc (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.cc...
media/filters/ffmpeg_demuxer.cc:354: int FFmpegDemuxer::Read(size_t size, uint8*
data) {
ditto ret val

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.cc...
media/filters/ffmpeg_demuxer.cc:391: return last_read_bytes;
I can't believe with all our clang magic we still allow this silent
truncation...

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.h
File media/filters/ffmpeg_demuxer.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.h#...
media/filters/ffmpeg_demuxer.h:149: // FFmpegProtocol implementation.
s/FFmpegProtocol/FFmpegURLProtocol/

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.h#...
media/filters/ffmpeg_demuxer.h:150: virtual int Read(size_t size, uint8* data)
OVERRIDE;
what about the return value?

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_glue.h
File media/filters/ffmpeg_glue.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_glue.h#new...
media/filters/ffmpeg_glue.h:48: virtual int Read(size_t size, uint8* data) = 0;
ret val

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
File media/filters/in_memory_url_protocol.cc (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:12: size_(size),
Clamp negative values?

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:19: int InMemoryUrlProtocol::Read(size_t
size, uint8* data) {
ret val

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:20: if (size < 0)
This check can go away.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:23: size_t available_bytes = size_ -
position_;
It'd be nice if size_>=position_ was an invariant of this class.
(seeing the result of a subtraction assigned to an unsigned type gives me the
heebie-jeebies)

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
File media/filters/in_memory_url_protocol.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:20: InMemoryUrlProtocol(const uint8* buf,
int64 size, bool streaming);
I wonder about the wisdom of having |size| not be a size_t as well.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:24: virtual int Read(size_t size, uint8*
data);
ret val

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:32: int64 size_;
...and here.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:33: int64 position_;
and here :)

[Cris Neckar, 2011-10-19 22:25:41]

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.cc
File media/filters/ffmpeg_demuxer.cc (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.cc...
media/filters/ffmpeg_demuxer.cc:354: int FFmpegDemuxer::Read(size_t size, uint8*
data) {
On 2011/10/19 21:14:58, Ami Fischman wrote:
> ditto ret val

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.h
File media/filters/ffmpeg_demuxer.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.h#...
media/filters/ffmpeg_demuxer.h:149: // FFmpegProtocol implementation.
On 2011/10/19 21:14:58, Ami Fischman wrote:
> s/FFmpegProtocol/FFmpegURLProtocol/

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_demuxer.h#...
media/filters/ffmpeg_demuxer.h:150: virtual int Read(size_t size, uint8* data)
OVERRIDE;
On 2011/10/19 21:14:58, Ami Fischman wrote:
> what about the return value?

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_glue.h
File media/filters/ffmpeg_glue.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/ffmpeg_glue.h#new...
media/filters/ffmpeg_glue.h:48: virtual int Read(size_t size, uint8* data) = 0;
On 2011/10/19 21:14:58, Ami Fischman wrote:
> ret val

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
File media/filters/in_memory_url_protocol.cc (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:12: size_(size),
On 2011/10/19 21:14:58, Ami Fischman wrote:
> Clamp negative values?

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:19: int InMemoryUrlProtocol::Read(size_t
size, uint8* data) {
On 2011/10/19 21:14:58, Ami Fischman wrote:
> ret val

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:20: if (size < 0)
On 2011/10/19 21:14:58, Ami Fischman wrote:
> This check can go away.

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.cc:23: size_t available_bytes = size_ -
position_;
On 2011/10/19 21:14:58, Ami Fischman wrote:
> It'd be nice if size_>=position_ was an invariant of this class.
> (seeing the result of a subtraction assigned to an unsigned type gives me the
> heebie-jeebies)

When I first saw this I was trying to find a way to make that happen and
couldn't but that isn't to say it couldn't be added later :)

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
File media/filters/in_memory_url_protocol.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:20: InMemoryUrlProtocol(const uint8* buf,
int64 size, bool streaming);
On 2011/10/19 21:14:58, Ami Fischman wrote:
> I wonder about the wisdom of having |size| not be a size_t as well.

This is a pretty major change that might have strange repercussions on 32 bit
platforms for example. It would also require far reaching changes throughout
filters which I feel are a good idea but perhaps would be best approached by
filing a new bug.

I really don't like that we are using signed types for these things :/

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:24: virtual int Read(size_t size, uint8*
data);
On 2011/10/19 21:14:58, Ami Fischman wrote:
> ret val

Done.

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:32: int64 size_;
On 2011/10/19 21:14:58, Ami Fischman wrote:
> ...and here.

see above

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:33: int64 position_;
On 2011/10/19 21:14:58, Ami Fischman wrote:
> and here :)

see above

[Ami GONE FROM CHROMIUM, 2011-10-19 23:32:41]

lgtm

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
File media/filters/in_memory_url_protocol.h (right):

http://codereview.chromium.org/8352019/diff/1/media/filters/in_memory_url_pro...
media/filters/in_memory_url_protocol.h:20: InMemoryUrlProtocol(const uint8* buf,
int64 size, bool streaming);
> I really don't like that we are using signed types for these things :/

Hah!  Whereas _I_ really don't like that we use *un*signed types for numbers :)