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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 5, 2010, 15:38 UTC
Message-ID: <20101005153838.GG12797@burratino>
In-Reply-To: <1285719811-10871-5-git-send-email-johan@herland.net>

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?