On 18.12.2012 07:30, Kinkie wrote:
On Mon, Nov 12, 2012 at 9:21 PM, Alex Rousskov
<[email protected]> wrote:
On 11/03/2012 05:55 AM, Kinkie wrote:
Feature-branch is at lp:~kinkie/squid/stringng
<snip>
+ message = static_cast<char*>(xmalloc(buf.length()+1));
+ buf.copy(message,buf.length());
+ message[buf.length()]='\0';
This code can be simplified by writing:
message = xstrdup(buf.c_str());
Er.. I don't think so: c_str() may cow(), which with the current code
doesn't happen.
It may be not really relevant as exceptions should not happen often.
What is more valuable to you? Performance or code brevity?
The key detail is that c_str() *MAY* cow() whereas the above code
*ALWAYS* copies to a new exception buffer.
cow() is fine in this case since:
1) it saves all the copies in those other cases where c_str() does not
cow().
2) an exception is happening, chances are high that we are going to
abort and drop the data, or edit the SBuf anyway (ie fix header syntax
values).
src/ipc/TypedMsgHdr.cc appears to have changes unrelated to this
project.
No changes appear appear with a file-based diff.. They were probably
performed in trunk but trunk wasn't merged yet.
src/mem.cc has whitespace non-changes.
Reverted to match trunk.
I assume you will drop StrList and friends from this submission so I
did
not continue to review them.
Yes. The only question is whether to leave what was done so far in
(#if 0-ed) or remove it.
I have not reviewed Tokenizer-related changes yet. I will wait for
all
other issues to settle first. Feel free to exclude Tokenizer from
the
proposed commit scope. It may be best to review Tokenizer when we
also
have some Squid code written or adjusted to use it.
Sure, Tokenizer is not yet used. The same question arises as with
StrList, whether I can leave it in (#if 0-ed) or I should cherrypick
it out.
If it is easy to cherry-pick SBuf into trunk and fix Tokenizer later I
think we should do that approach simply to get the acceptable working
bits into trunk.
If that is not easy, IMHO we should audit and add both. They are both
needed in the end product anyway.
OR, can Tokenizer work with SquidString / MemBuf / StringArea and go
in first?
Amos
