From: Thiago Farina Date: Sun, 14 Nov 2010 20:46:36 GMT Subject: Re: [PATCHv6 06/23] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond Message-ID: In-Reply-To: <1289339399-4733-7-git-send-email-johan@herland.net> On Tue, Nov 9, 2010 at 7:49 PM, 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(). > > Instead, propagate this return value out to add_note(), and return it > from there to enable the caller to handle errors appropriately. > > Existing add_note() callers are updated to die() upon failure, thus > preserving the current behaviour. The only exceptions are copy_note() > and notes_cache_put() where we are able to propagate the add_note() > return value instead. > > This patch has been improved by the following contributions: > - Jonathan Nieder: Future-proof by always checking add_note() return value > - Jonathan Nieder: Improve clarity of final if-condition in note_tree_insert() > > Thanks-to: Jonathan Nieder > Signed-off-by: Johan Herland > --- >  builtin/notes.c |   11 ++++++----- >  notes-cache.c   |    3 +-- >  notes.c         |   55 ++++++++++++++++++++++++++++--------------------------- >  notes.h         |   11 ++++++++--- >  4 files changed, 43 insertions(+), 37 deletions(-) > > diff --git a/builtin/notes.c b/builtin/notes.c > index fbc347c..35f6eb6 100644 > --- a/builtin/notes.c > +++ b/builtin/notes.c > @@ -573,8 +573,8 @@ static int add(int argc, const char **argv, const char *prefix) > >        if (is_null_sha1(new_note)) >                remove_note(t, object); > -       else > -               add_note(t, object, new_note, combine_notes_overwrite); > +       else if (add_note(t, object, new_note, combine_notes_overwrite)) > +               die("confused: combine_notes_overwrite failed"); This message looks like more a debug information. Could it be rewritten to be more user friendly / more informative?