[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH] strbuf_readlink semantics update.
From:       Pierre Habouzit <madcoder () debian ! org>
Date:       2009-01-04 12:21:08
Message-ID: 20090104122108.GC29325 () artemis ! corp
[Download RAW message or body]

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

[Attachment #3 (application/pgp-signature)]
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[prev in list] [next in list] [prev in thread] [next in thread]