Re: [PATCHv6 06/23] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
- From
Thiago Farina <tfransosi@gmail.com>
- Date
- Nov 14, 2010, 20:46 UTC
- Message-ID
- <AANLkTinQhkXvfa0BS0dFK6-mXFpU+uQ=rKRJ1QaXXA1S@mail.gmail.com>
- In-Reply-To
- <1289339399-4733-7-git-send-email-johan@herland.net>
On Tue, Nov 9, 2010 at 7:49 PM, Johan Herland <johan@herland.net> wrote:
Show 37 quoted lines
> 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 <jrnieder@gmail.com>
> Signed-off-by: Johan Herland <johan@herland.net>
> ---
> 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?