Re: [PATCHv4 06/21] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond

On Thursday 21 October 2010, Jonathan Nieder wrote:

Johan Herland wrote:

quoted

--- a/notes.c
+++ b/notes.c

@@ -303,13 +298,17 @@ static void note_tree_insert(struct

notes_tree *t, struct int_node *tree, GET_PTR_TYPE(*p) ==
PTR_TYPE_SUBTREE);
 	if (is_null_sha1(entry->val_sha1)) { /* skip insertion of empty
note */ free(entry);
-		return;
+		return 0;
 	}
 	new_node = (struct int_node *) xcalloc(sizeof(struct int_node),
1); -	note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
-			 combine_notes);
-	*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
-	note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
+	ret = note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
+			       combine_notes);
+	if (!ret) {
+		*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
+		ret = note_tree_insert(t, new_node, n + 1, entry, type,
+				       combine_notes);
+	}
+	return ret;

Micronit: it would probably be clearer to write

	if (ret)
		return ret;
	*p = SET_PTR_TYPE(...
	return note_tree_insert(...

to avoid a little nesting.

Agreed. Will be fixed in the next iteration.


Thanks,

...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net