Re: StringNG merge?

On 4/11/2012 1:56 a.m., Amos Jeffries wrote:
> On 4/11/2012 12:55 a.m., Kinkie wrote:
>> Hi all,
>> Is it maybe time to resuscitate StringNG, with the target of merging
>> in time for 3.3?
>
> It is too late for 3.3 features and polish now.
> But 3.4 is not scheduled for branching until next March and StringNG
> should be able to make that IMO.
>
>> I have unrotten the code, which now compiles and passes the unit
>> tests, with Amos' recent changes to RefCountable/Lockable.
>> The only API objection I remember (SBuf::terminate() being public) was
>> fixed some time ago -
>
> I had an objection that terminate() as a function was not necessary at
> all.
> I want to check my patch against the current code and complete that
> discussion before merge.
>
>> I guess that the code was ready for merging but
>> held off due to merge window being closed.
>
> Uhm, we have no "merge windows" as such. The periods when I need no
> commits is just a few minutes on the branching dates. After which the
> branch is all that closes, but trunk remains open for merges and
> commits at all other times.
>
>> Only recent change is the renaming of SBuf::findAny to find_first_of
>> for consistency with std::string.
>> Feature-branch is at lp:~kinkie/squid/stringng
>>
>> If we agree, any preference on how to proceed?
>
> Waiting for that agreement. Then merge.
>
> I should have time over the next day to check the bits I want to check.
>
>
> Amos

Nits: (please fix before merge, but no extra review needed after changing)
  * can you remove the extra whitespace around function name and
paramater list '(' characters please.
   ** I am seeing a bunch of "foo( type" and foo (type" variantions on
methods.

The terminate() problem I want to get rid of is still there.... my
objection is that we do not actually need it to exist as a visible
function at all.

1) line 707 please use c_str() instead of buf() to access the
nul-terminated buffer.
    c_str() produces a terminated buffer, so line 706 call to
terminate() is now redundant.

   * If you want to do "--stats.rawAccess" in SBuf::scanf() to offset
the rawAccess counting c_str() does, fine. But I'm inclined to think of
*scanf() as a bad thing to call anyway. The inducement to remove it
could be helpful.

  ... which leaves one *single* use of terminate() at line 500.

2) explicitly inline the terminate() code at line 500.

3) remove the unused symbol SBuf::terminate().

This hides the terminate() availability from public code documentation,
pushing people towards c_str() instead which is what we want to do.
It also inflates the rawAccess counter stats to show that vsscanf()
operations are working on the raw buffer in a bad way and should be
avoided when possible.

Amos
Received on Sun Nov 04 2012 - 00:30:24 MDT