>> But what you sort of want is a forced-64-bit size_t. ("size64_t" ?) This
seems
>> like a valid argument for using uint64 for these args, even though it's
>> otherwise frowned upon by Google style. And after doing that, many of these
>> Blob diffs could be reverted, I think?
>
> Eh? "size64_t"!?? Either you want int64 or uint64, or you want ssize_t or
> size_t? (ssize_t is the signed equivalent of size_t).
>
> As for 32-bit versus 64-bit, Blobs have to be 64-bit, since files can be
larger
> than 4GB.
>
> As for signed versus unsigned, it's less clear. I'm still leaning towards
> signed, partly because JavaScript doesn't have a notion of unsigned, and
partly
> because we use negative numbers for error codes (e.g. a failed Read() returns
> -1).
>
> As you've noted, there's a bunch of diffs in this CL to cope with the
> signed/unsigned mismatch (in particular, Blobs speak signed, std::vector
speaks
> size_t). If we wanted to change Blobs to speak unsigned, then there will
> certainly be changes in this CL that we could remove, but there very well
could
> be other changes that we'd have to add, where the code currently assumes
> signed-ness.
>
> Finally, leaving things as the status quo (i.e. signed) is a smaller change,
and
> smaller changes are usually better changes.
My specific concern is that the new code is doing sanity checks, during
conversion, that are pretty non-obvious to me. If these continue to be
scattered through the code, somebody is going to forget to do them at some
point.
If you had conversion functions or sanity-checking functions, which hid the
details from callers, that would alleviate my concern.
You're a better judge of what minimal set of operations you would need. But
some examples might be:
* int64 CastToBlobSize(size_t) // returns -1 on error
* int BlobSizeCompare(size_t size1, int64 size2) // returns negative/zero/pos
And these would hide all the error checking / assertions.
========================================================================
http://mondrian.corp.google.com/file/10624727///depot/googleclient/gears/opensource/gears/base/firefox/install.rdf.m4?a=1
File //depot/googleclient/gears/opensource/gears/base/firefox/install.rdf.m4
(snapshot 1)
------------------------------------
Line 83: <em:targetPlatform>Linux_x86_64-gcc3</em:targetPlatform>
On 11:35 pm, 29 Mar, nigeltao wrote:
> On 4:19 pm, cprince wrote:
> > I wonder if this should set <targetPlatform> too, for consistency?
> > If there was some good reason not to, that's fine too.
>
> I'm not sure what you mean -- I am setting <em:targetPlatform>, and I don't
> think I can drop the "em". The 32-bit Linux Gears installer sets
> <em:targetPlatform> twice (to quirk for Ubuntu Edgy 6.10), but that doesn't
seem
> to be needed for Ubunty Hardy 8.04.
Ah, sorry - I was getting confused by how Ubunto 6.10 required a 2nd
targetPlatform value.
========================================================================
--
To respond, reply to this email or visit
http://mondrian.corp.google.com/10624727
