Re: [PATCH] strbuf_readlink semantics update.

On Thu, Dec 25, 2008 at 07:23:58AM +0000, Junio C Hamano wrote:

René Scharfe [off-list ref] writes:

quoted

Pierre Habouzit schrieb:

quoted

On Tue, Dec 23, 2008 at 06:16:01PM +0000, Linus Torvalds wrote:

quoted

On Tue, 23 Dec 2008, Pierre Habouzit wrote:

quoted

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.

I agree it's a fine semantics.

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.

Ack.

Sorry for the delay I was on vacation.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org