Subject: Re: [PATCH] strbuf_readlink semantics update.

From: Junio C Hamano <[email-masked]>
Date: Wed, 24 Dec 2008 23:23:58 -0800

René Scharfe <[email-masked]> writes:

> Pierre Habouzit schrieb:
>> On Tue, Dec 23, 2008 at 06:16:01PM +0000, Linus Torvalds wrote:
>>>
>>> On Tue, 23 Dec 2008, Pierre Habouzit wrote:
>>>> when readlink fails, the strbuf shall not be destroyed. It's not how
>>>> read_file_or_gitlink works for example.
>>> I disagree.
>>>
>>> This patch just makes things worse. Just leave the "strbuf_release()" in 
>>> _one_ place.
> ...
> The "append or do nothing" rule is broken by strbuf_getline(), but I agree
> to your reasoning.  How about refining this rule a bit to "do your thing
> and roll back changes if an error occurs"?  I think it's not worth to undo
> allocation extensions, but making reverting first time allocations seems
> like a good idea.  Something like this?

I think this is much better than Pierre's.  Pierre's "if it is called
strbuf_*, it should always append" is a good uniformity to have in an API,
but making the caller suffer for clean-up is going backwards.  The reason
we use strbuf when we can is so that the callers do not have to worry
about memory allocation issues too much.