Re: [PATCH 04/18] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond

> 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?