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 "problem" is that the strbuf API usually works that way: functions
append things to a buffer, or do nothing, but always keep the buffer in
a state where you can append more stuff to it.

If read_file_or_gitlink or strbuf_readlink destroy the buffer, then you
break the second expectation people (should) have about the strbuf API.

The reason is that if you built things in the buffer, you really don't
want it to be undone just because the last bit you add went wrong for
some reason. Or if you have a buffer that is reused in a loop, you don't
want the buffer you allocated to be dropped just because one error
occurred.


Alternatively, we could pass a flag to tell function performing reads
(fread, read, readlink, whatever) that those should destroy the buffer
on error or just report it. I don't really know. It sounds like
over-engineering though.


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