notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johan Herland committed
Nov 15, 2010 at 00:52 UTC
180619a58572b17de0ebeb96989e0aa832765186
4 files changed
+43
-37
builtin/notes.c
+6
-5
@@ -573,8 +573,8 @@ static int add(int argc, const char **argv, const char *prefix)
573
574
if (is_null_sha1(new_note))
575
remove_note(t, object);
576
- else
577
- add_note(t, object, new_note, combine_notes_overwrite);
576
+ else if (add_note(t, object, new_note, combine_notes_overwrite))
577
+ die("BUG: combine_notes_overwrite failed");
578
579
snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
580
is_null_sha1(new_note) ? "removed" : "added", "add");
@@ -653,7 +653,8 @@ static int copy(int argc, const char **argv, const char *prefix)
653
goto out;
654
}
655
656
- add_note(t, object, from_note, combine_notes_overwrite);
656
+ if (add_note(t, object, from_note, combine_notes_overwrite))
657
+ die("BUG: combine_notes_overwrite failed");
658
commit_notes(t, "Notes added by 'git notes copy'");
659
out:
660
free_notes(t);
@@ -712,8 +713,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
713
714
if (is_null_sha1(new_note))
715
remove_note(t, object);
715
- else
716
- add_note(t, object, new_note, combine_notes_overwrite);
716
+ else if (add_note(t, object, new_note, combine_notes_overwrite))
717
+ die("BUG: combine_notes_overwrite failed");
718
719
snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
720
is_null_sha1(new_note) ? "removed" : "added", argv[0]);
notes-cache.c
+1
-2
@@ -89,6 +89,5 @@ int notes_cache_put(struct notes_cache *c, unsigned char key_sha1[20],
89
90
if (write_sha1_file(data, size, "blob", value_sha1) < 0)
91
return -1;
92
- add_note(&c->tree, key_sha1, value_sha1, NULL);
93
- return 0;
92
+ return add_note(&c->tree, key_sha1, value_sha1, NULL);
93
}
notes.c
+28
-27
@@ -235,13 +235,14 @@ static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
235
* - Else, create a new int_node, holding both the node-at-location and the
236
* node-to-be-inserted, and store the new int_node into the location.
237
*/
238
-static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
238
+static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
239
unsigned char n, struct leaf_node *entry, unsigned char type,
240
combine_notes_fn combine_notes)
241
{
242
struct int_node *new_node;
243
struct leaf_node *l;
244
void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
245
+ int ret = 0;
246
247
assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
248
l = (struct leaf_node *) CLR_PTR_TYPE(*p);
@@ -252,26 +253,21 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
253
free(entry);
254
else
255
*p = SET_PTR_TYPE(entry, type);
255
- return;
256
+ return 0;
257
case PTR_TYPE_NOTE:
258
switch (type) {
259
case PTR_TYPE_NOTE:
260
if (!hashcmp(l->key_sha1, entry->key_sha1)) {
261
/* skip concatenation if l == entry */
262
if (!hashcmp(l->val_sha1, entry->val_sha1))
262
- return;
263
+ return 0;
264
264
- if (combine_notes(l->val_sha1, entry->val_sha1))
265
- die("failed to combine notes %s and %s"
266
- " for object %s",
267
- sha1_to_hex(l->val_sha1),
268
- sha1_to_hex(entry->val_sha1),
269
- sha1_to_hex(l->key_sha1));
270
-
271
- if (is_null_sha1(l->val_sha1))
265
+ ret = combine_notes(l->val_sha1,
266
+ entry->val_sha1);
267
+ if (!ret && is_null_sha1(l->val_sha1))
268
note_tree_remove(t, tree, n, entry);
269
free(entry);
274
- return;
270
+ return ret;
271
}
272
break;
273
case PTR_TYPE_SUBTREE:
@@ -280,7 +276,7 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
276
/* unpack 'entry' */
277
load_subtree(t, entry, tree, n);
278
free(entry);
283
- return;
279
+ return 0;
280
}
281
break;
282
}
@@ -291,9 +287,8 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
287
*p = NULL;
288
load_subtree(t, l, tree, n);
289
free(l);
294
- note_tree_insert(t, tree, n, entry, type,
295
- combine_notes);
296
- return;
290
+ return note_tree_insert(t, tree, n, entry, type,
291
+ combine_notes);
292
}
293
break;
294
}
@@ -303,13 +298,15 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
298
GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
299
if (is_null_sha1(entry->val_sha1)) { /* skip insertion of empty note */
300
free(entry);
306
- return;
301
+ return 0;
302
}
303
new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
309
- note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
310
- combine_notes);
304
+ ret = note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
305
+ combine_notes);
306
+ if (ret)
307
+ return ret;
308
*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
312
- note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
309
+ return note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
310
}
311
312
/* Free the entire notes data contained in the given tree */
@@ -452,8 +449,12 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
449
l->key_sha1[19] = (unsigned char) len;
450
type = PTR_TYPE_SUBTREE;
451
}
455
- note_tree_insert(t, node, n, l, type,
456
- combine_notes_concatenate);
452
+ if (note_tree_insert(t, node, n, l, type,
453
+ combine_notes_concatenate))
454
+ die("Failed to load %s %s into notes tree "
455
+ "from %s",
456
+ type == PTR_TYPE_NOTE ? "note" : "subtree",
457
+ sha1_to_hex(l->key_sha1), t->ref);
458
}
459
continue;
460
@@ -1014,7 +1015,7 @@ void init_display_notes(struct display_notes_opt *opt)
1015
string_list_clear(&display_notes_refs, 0);
1016
}
1017
1017
-void add_note(struct notes_tree *t, const unsigned char *object_sha1,
1018
+int add_note(struct notes_tree *t, const unsigned char *object_sha1,
1019
const unsigned char *note_sha1, combine_notes_fn combine_notes)
1020
{
1021
struct leaf_node *l;
@@ -1028,7 +1029,7 @@ void add_note(struct notes_tree *t, const unsigned char *object_sha1,
1029
l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
1030
hashcpy(l->key_sha1, object_sha1);
1031
hashcpy(l->val_sha1, note_sha1);
1031
- note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
1032
+ return note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
1033
}
1034
1035
void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
@@ -1204,7 +1205,7 @@ void format_display_notes(const unsigned char *object_sha1,
1205
1206
int copy_note(struct notes_tree *t,
1207
const unsigned char *from_obj, const unsigned char *to_obj,
1207
- int force, combine_notes_fn combine_fn)
1208
+ int force, combine_notes_fn combine_notes)
1209
{
1210
const unsigned char *note = get_note(t, from_obj);
1211
const unsigned char *existing_note = get_note(t, to_obj);
@@ -1213,9 +1214,9 @@ int copy_note(struct notes_tree *t,
1214
return 1;
1215
1216
if (note)
1216
- add_note(t, to_obj, note, combine_fn);
1217
+ return add_note(t, to_obj, note, combine_notes);
1218
else if (existing_note)
1218
- add_note(t, to_obj, null_sha1, combine_fn);
1219
+ return add_note(t, to_obj, null_sha1, combine_notes);
1220
1221
return 0;
1222
}
notes.h
+8
-3
@@ -104,11 +104,13 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
104
* note with the empty note (using the given combine_notes function) results
105
* in a new/changed note.
106
*
107
+ * Returns zero on success; non-zero means combine_notes failed.
108
+ *
109
* IMPORTANT: The changes made by add_note() to the given notes_tree structure
110
* are not persistent until a subsequent call to write_notes_tree() returns
111
* zero.
112
*/
111
-void add_note(struct notes_tree *t, const unsigned char *object_sha1,
113
+int add_note(struct notes_tree *t, const unsigned char *object_sha1,
114
const unsigned char *note_sha1, combine_notes_fn combine_notes);
115
116
/*
@@ -131,7 +133,10 @@ const unsigned char *get_note(struct notes_tree *t,
133
/*
134
* Copy a note from one object to another in the given notes_tree.
135
*
134
- * Fails if the to_obj already has a note unless 'force' is true.
136
+ * Returns 1 if the to_obj already has a note and 'force' is false. Otherwise,
137
+ * returns non-zero if 'force' is true, but the given combine_notes function
138
+ * failed to combine from_obj's note with to_obj's existing note.
139
+ * Returns zero on success.
140
*
141
* IMPORTANT: The changes made by copy_note() to the given notes_tree structure
142
* are not persistent until a subsequent call to write_notes_tree() returns
@@ -139,7 +144,7 @@ const unsigned char *get_note(struct notes_tree *t,
144
*/
145
int copy_note(struct notes_tree *t,
146
const unsigned char *from_obj, const unsigned char *to_obj,
142
- int force, combine_notes_fn combine_fn);
147
+ int force, combine_notes_fn combine_notes);
148
149
/*
150
* Flags controlling behaviour of for_each_note()