Re: StringNG merge?

Hi,
  I've implemented all your suggestions.

On Sun, Nov 4, 2012 at 1:30 AM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> 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

-- 
    /kinkie