On Thu, Dec 25, 2008 at 07:23:58AM +0000, Junio C Hamano wrote: > René Scharfe 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. 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