From: Johan Herland <johan@herland.net>
Date: Wed, 06 Oct 2010 19:40:56 GMT
Subject: Re: [PATCH 04/18] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
Message-ID: <201010062140.56999.johan@herland.net>
In-Reply-To: <20101005153838.GG12797@burratino>

On Tuesday 5. October 2010 17.38.38 Jonathan Nieder wrote:
> Johan Herland wrote:
> > The combine_notes_fn functions uses a non-zero return value to indicate
> > failure. However, this return value was converted to a call to die()
> > in note_tree_insert().
> 
> For what it's worth, I still like it.
> 
> > Instead, propagate this return value out to add_note(), and return it
> > from there
> 
> It's always odd deciding what to do in the code paths that "can't
> happen".  Ideally one would want static analyzers to check that, while
> at the same time subjecting the user to some nice graceful fallback
> behavior when it happens anyway.
> 
> 	fatal: confused: combine_notes_overwrite failed
> 
> In this case I'm not sure what the message in die() should be, but
> it seems sane to die with some message.  Maybe this should be
> mentioned in the commit message, though?

Ok, I've added a paragraph to the commit message mentioning that most of 
add_note()'s callers now implement the die() previously located in 
notes_tree_insert(). Will be in the next iteration.


Thanks,

...Johan


-- 
Johan Herland, <johan@herland.net>
www.herland.net