Re: [PATCH] Rework strbuf API and semantics.

From: Andreas Ericsson <ae@op5.se>
Date: Sep 4, 2007, 13:34 UTC
Message-ID: <46DD5EF4.3090309@op5.se>
In-Reply-To: <20070904115317.GA3381@artemis.corp>

Pierre Habouzit wrote:

Show 18 quoted lines

> On Tue, Sep 04, 2007 at 11:11:45AM +0000, Johannes Schindelin wrote:
>> Hi,
>>
>> On Tue, 4 Sep 2007, Pierre Habouzit wrote:
>>
>> - IMHO the same goes for strbuf_free() instead of strbuf_wipe(), and
> 
>   Well, I don't like strbuf_free, because it's opposed to
> strbuf_alloc/new whatever, that would be functions that create and
> release a struct strbuf * (meaning allocating the struct shell as well).
> Here you only free the internal buffer, not the shell, so _free() would
> be a very bad name.
> 
>   In my own coding rules, I have _new/_delete and _init/_wipe functions,
> the former acts on pointers, the latter on the structs. Hence the
> naming. Though, looking at git's code, it seems that the usual name for
> this operation is _release. So would you go for strbuf_release ?
>

release is indeed better. To me, "wipe" means zeroing out (possibly after an optional number of passes writing trash to the memory area) rather than actually freeing any memory.

Show 15 quoted lines

> 
>> - it would be nice to split this patch into
>>
>> 	- the API change (with _minimal_ changes to anything outside of 
>> 	  strbuf.[ch]), and
>>
>> 	- the cleanups in the rest of the code.
> 
>   I'll try to do that, but it's quite hard to achieve knowing that in
> many places of the current state of master, there are embeded NUL's
> (accounted in ->len). I'll try to see what I can split, but it's
> unlikely I'll be able to have an intermediate _working_ state (I mean
> that would pass the testsuite) with the new API/semantics _and_ without
> touching a lot less of the rest of the code.
>

Always having a compilable/operational tree is pretty nice primarily due to bisect. If the code turns from working to non-working in between, make sure to jot it down in the commit message.

It might even be better to add the embedded NUL handling code in the first patch, modify the callers in the second, and remove the embedded NUL handling in a third.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231