Re: [RFC] Time to talk about StringNG merge again?

-- This is not a review, but I wanted to clarify or emphasize some of
the points in Amos' review. Thank you Amos for working on this!

On 03/30/2013 02:32 AM, Amos Jeffries wrote:
> in src/SBuf.cc:
>
> * SBuf::chop() - the TODO optimization is not possible. We cannot be
> completely sure we are teh only user of the

I think it is possible, but the MemBlob cooperation would be required to
ensure correctness.

> * SBuf::operator []() documentation is incorrect. It *does* check access
> bounds, but returns \0 for any bytes outside instead of throwing exception.

The documentation is correct. The implementation is no longer correct.
Please revert to the recent code version that did none of the above. The
[] operator is meant for performance-sensitive loops and should not do
any checks. Even the stats collection should be removed from it IMO. We
provide at() that trades speed for safety. std::string does the same.

> in src/SBuf.cci

BTW, IIRC, we discussed moving all .cci stuff into .h. Or was that a
different project?

> * Sbuf::bufEnd() is lacking any checks about whether the space returned
> is safe to use, no cow() or whatever.

bufEnd() is a private convenience method. It is unsafe and non-cowing by
design. The callers should check and cow, as needed. If the
documentation does not reflect that, it should be fixed.

Thank you,

Alex.
Received on Mon Apr 01 2013 - 17:57:31 MDT