On Tue, Mar 31, 2009 at 17:27, Chris Prince <[email protected]> wrote: > 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.
People might not forget to do so, given that leaving out a static_cast is a compile error (on 64-bit Ubuntu Hardy). True, we don't have a 64-bit Ubuntu build-bot yet, but getting to build once is the first step to getting it to build continuously. > 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. I think that the size_t versus int64 issue only really bites us for BufferBlobs, not for any other sorts of Blobs. BufferBlobs use a std::vector, which is size_t, whereas other Blobs generally speak int64. I'm not sure if adding a functional layer of indirection just for buffer_blob.cc would gain much, if anything, since buffer_blob.cc is a small file (109 lines of code, including a copyright header), and e.g. CastToBlobSize would just be an inline static_cast.
