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

[Johan Herland <johan@herland.net>]

On Friday 23 July 2010, Jonathan Nieder wrote:
> Johan Herland wrote:
> > +		/* No return value checking; c_n_overwrite always returns 0 */
> >  		add_note(t, object, new_note, combine_notes_overwrite);
> 
> I suspect something like
> 
> 	if (add_note(t, object, ...))
> 		die("confused: combine_notes_overwrite failed");
> 
> would be less likely to fall out of date. ;-)

Good call, I'll squash the following into the next iteration:

Thanks!

...Johan


diff --git a/builtin/notes.c b/builtin/notes.c
index 516401c..5aaae03 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -576,9 +576,8 @@ static int add(int argc, const char **argv, const char *prefix)
 
 	if (is_null_sha1(new_note))
 		remove_note(t, object);
-	else
-		/* No return value checking; c_n_overwrite always returns 0 */
-		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");
 
 	snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
 		 is_null_sha1(new_note) ? "removed" : "added", "add");
@@ -657,8 +656,8 @@ static int copy(int argc, const char **argv, const char *prefix)
 		goto out;
 	}
 
-	/* No return value checking; c_n_overwrite always returns 0 */
-	add_note(t, object, from_note, combine_notes_overwrite);
+	if (add_note(t, object, from_note, combine_notes_overwrite))
+		die("confused: combine_notes_overwrite failed");
 	commit_notes(t, "Notes added by 'git notes copy'");
 out:
 	free_notes(t);
@@ -717,9 +716,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 
 	if (is_null_sha1(new_note))
 		remove_note(t, object);
-	else
-		/* No return value checking; c_n_overwrite always returns 0 */
-		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");
 
 	snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
 		 is_null_sha1(new_note) ? "removed" : "added", argv[0]);
diff --git a/notes-merge.c b/notes-merge.c
index 122d6b9..47cd32a 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -442,12 +442,14 @@ static int merge_one_change(struct notes_merge_options *o,
 		return 0;
 	case NOTES_MERGE_RESOLVE_THEIRS:
 		OUTPUT(o, 2, "Using remote notes for %s", sha1_to_hex(p->obj));
-		add_note(t, p->obj, p->remote, combine_notes_overwrite);
+		if (add_note(t, p->obj, p->remote, combine_notes_overwrite))
+			die("confused: combine_notes_overwrite failed");
 		return 0;
 	case NOTES_MERGE_RESOLVE_UNION:
 		OUTPUT(o, 2, "Concatenating local and remote notes for %s",
 		       sha1_to_hex(p->obj));
-		add_note(t, p->obj, p->remote, combine_notes_concatenate);
+		if (add_note(t, p->obj, p->remote, combine_notes_concatenate))
+			die("confused: combine_notes_concatenate failed");
 		return 0;
 	}
 	die("Unknown resolve method (%i).", o->resolve);
@@ -489,7 +491,9 @@ static int merge_changes(struct notes_merge_options *o,
 			   !hashcmp(p->local, p->base)) {
 			/* no local change; adopt remote change */
 			OUTPUT(o, 5, "\t\t\tno local change, adopting remote");
-			add_note(t, p->obj, p->remote, combine_notes_overwrite);
+			if (add_note(t, p->obj, p->remote,
+				     combine_notes_overwrite))
+				die("confused: combine_notes_overwrite failed");
 		} else {
 			/* need file-level merge between local and remote */
 			OUTPUT(o, 5, "\t\t\tneed content-level merge");


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