From: Johan Herland <johan@herland.net>
To: git@vger.kernel.org
Cc: johan@herland.net, jrnieder@gmail.com, bebarino@gmail.com,
avarab@gmail.com, gitster@pobox.com
Subject: [PATCHv4 06/21] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
Date: Thu, 21 Oct 2010 04:08:41 +0200 [thread overview]
Message-ID: <1287626936-32232-7-git-send-email-johan@herland.net> (raw)
In-Reply-To: <1287626936-32232-1-git-send-email-johan@herland.net>
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
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 | 59 ++++++++++++++++++++++++++++--------------------------
notes.h | 11 +++++++--
4 files changed, 46 insertions(+), 38 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");
snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
is_null_sha1(new_note) ? "removed" : "added", "add");
@@ -653,7 +653,8 @@ static int copy(int argc, const char **argv, const char *prefix)
goto out;
}
- 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);
@@ -712,8 +713,8 @@ static int append_edit(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");
snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
is_null_sha1(new_note) ? "removed" : "added", argv[0]);
diff --git a/notes-cache.c b/notes-cache.c
index dee6d62..4c8984e 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -89,6 +89,5 @@ int notes_cache_put(struct notes_cache *c, unsigned char key_sha1[20],
if (write_sha1_file(data, size, "blob", value_sha1) < 0)
return -1;
- add_note(&c->tree, key_sha1, value_sha1, NULL);
- return 0;
+ return add_note(&c->tree, key_sha1, value_sha1, NULL);
}
diff --git a/notes.c b/notes.c
index 0c13a36..c4203ce 100644
--- a/notes.c
+++ b/notes.c
@@ -235,13 +235,14 @@ static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
* - Else, create a new int_node, holding both the node-at-location and the
* node-to-be-inserted, and store the new int_node into the location.
*/
-static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
+static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
unsigned char n, struct leaf_node *entry, unsigned char type,
combine_notes_fn combine_notes)
{
struct int_node *new_node;
struct leaf_node *l;
void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
+ int ret = 0;
assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
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,
free(entry);
else
*p = SET_PTR_TYPE(entry, type);
- return;
+ return 0;
case PTR_TYPE_NOTE:
switch (type) {
case PTR_TYPE_NOTE:
if (!hashcmp(l->key_sha1, entry->key_sha1)) {
/* skip concatenation if l == entry */
if (!hashcmp(l->val_sha1, entry->val_sha1))
- return;
+ return 0;
- if (combine_notes(l->val_sha1, entry->val_sha1))
- die("failed to combine notes %s and %s"
- " for object %s",
- sha1_to_hex(l->val_sha1),
- sha1_to_hex(entry->val_sha1),
- sha1_to_hex(l->key_sha1));
-
- if (is_null_sha1(l->val_sha1))
+ ret = combine_notes(l->val_sha1,
+ entry->val_sha1);
+ if (!ret && is_null_sha1(l->val_sha1))
note_tree_remove(t, tree, n, entry);
free(entry);
- return;
+ return ret;
}
break;
case PTR_TYPE_SUBTREE:
@@ -280,7 +276,7 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
/* unpack 'entry' */
load_subtree(t, entry, tree, n);
free(entry);
- return;
+ return 0;
}
break;
}
@@ -291,9 +287,8 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
*p = NULL;
load_subtree(t, l, tree, n);
free(l);
- note_tree_insert(t, tree, n, entry, type,
- combine_notes);
- return;
+ return note_tree_insert(t, tree, n, entry, type,
+ combine_notes);
}
break;
}
@@ -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;
}
/* Free the entire notes data contained in the given tree */
@@ -452,8 +451,12 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
l->key_sha1[19] = (unsigned char) len;
type = PTR_TYPE_SUBTREE;
}
- note_tree_insert(t, node, n, l, type,
- combine_notes_concatenate);
+ if (note_tree_insert(t, node, n, l, type,
+ combine_notes_concatenate))
+ die("Failed to load %s %s into notes tree "
+ "from %s",
+ type == PTR_TYPE_NOTE ? "note" : "subtree",
+ sha1_to_hex(l->key_sha1), t->ref);
}
continue;
@@ -1014,7 +1017,7 @@ void init_display_notes(struct display_notes_opt *opt)
string_list_clear(&display_notes_refs, 0);
}
-void add_note(struct notes_tree *t, const unsigned char *object_sha1,
+int add_note(struct notes_tree *t, const unsigned char *object_sha1,
const unsigned char *note_sha1, combine_notes_fn combine_notes)
{
struct leaf_node *l;
@@ -1028,7 +1031,7 @@ void add_note(struct notes_tree *t, const unsigned char *object_sha1,
l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
hashcpy(l->key_sha1, object_sha1);
hashcpy(l->val_sha1, note_sha1);
- note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
+ return note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
}
void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
@@ -1204,7 +1207,7 @@ void format_display_notes(const unsigned char *object_sha1,
int copy_note(struct notes_tree *t,
const unsigned char *from_obj, const unsigned char *to_obj,
- int force, combine_notes_fn combine_fn)
+ int force, combine_notes_fn combine_notes)
{
const unsigned char *note = get_note(t, from_obj);
const unsigned char *existing_note = get_note(t, to_obj);
@@ -1213,9 +1216,9 @@ int copy_note(struct notes_tree *t,
return 1;
if (note)
- add_note(t, to_obj, note, combine_fn);
+ return add_note(t, to_obj, note, combine_notes);
else if (existing_note)
- add_note(t, to_obj, null_sha1, combine_fn);
+ return add_note(t, to_obj, null_sha1, combine_notes);
return 0;
}
diff --git a/notes.h b/notes.h
index 79ea797..b372575 100644
--- a/notes.h
+++ b/notes.h
@@ -104,11 +104,13 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
* note with the empty note (using the given combine_notes function) results
* in a new/changed note.
*
+ * Returns zero on success; non-zero means combine_notes failed.
+ *
* IMPORTANT: The changes made by add_note() to the given notes_tree structure
* are not persistent until a subsequent call to write_notes_tree() returns
* zero.
*/
-void add_note(struct notes_tree *t, const unsigned char *object_sha1,
+int add_note(struct notes_tree *t, const unsigned char *object_sha1,
const unsigned char *note_sha1, combine_notes_fn combine_notes);
/*
@@ -131,7 +133,10 @@ const unsigned char *get_note(struct notes_tree *t,
/*
* Copy a note from one object to another in the given notes_tree.
*
- * Fails if the to_obj already has a note unless 'force' is true.
+ * Returns 1 if the to_obj already has a note and 'force' is false. Otherwise,
+ * returns non-zero if 'force' is true, but the given combine_notes function
+ * failed to combine from_obj's note with to_obj's existing note.
+ * Returns zero on success.
*
* IMPORTANT: The changes made by copy_note() to the given notes_tree structure
* 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,
*/
int copy_note(struct notes_tree *t,
const unsigned char *from_obj, const unsigned char *to_obj,
- int force, combine_notes_fn combine_fn);
+ int force, combine_notes_fn combine_notes);
/*
* Flags controlling behaviour of for_each_note()
--
1.7.3.98.g5ad7d9
next prev parent reply other threads:[~2010-10-21 2:10 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-21 2:08 [PATCHv4 00/21] git notes merge Johan Herland
2010-10-21 2:08 ` [PATCHv4 01/21] notes.c: Hexify SHA1 in die() message from init_notes() Johan Herland
2010-10-21 2:08 ` [PATCHv4 02/21] (trivial) notes.h: Minor documentation fixes to copy_notes() Johan Herland
2010-10-21 2:08 ` [PATCHv4 03/21] notes.h: Make default_notes_ref() available in notes API Johan Herland
2010-10-21 2:08 ` [PATCHv4 04/21] notes.c: Reorder functions in preparation for next commit Johan Herland
2010-10-21 2:08 ` [PATCHv4 05/21] notes.h/c: Clarify the handling of notes objects that are == null_sha1 Johan Herland
2010-10-21 5:12 ` Jonathan Nieder
2010-10-21 13:13 ` Johan Herland
2010-10-21 17:54 ` Jonathan Nieder
2010-10-22 13:32 ` Johan Herland
2010-10-21 2:08 ` Johan Herland [this message]
2010-10-21 5:21 ` [PATCHv4 06/21] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond Jonathan Nieder
2010-10-21 13:16 ` Johan Herland
2010-10-21 2:08 ` [PATCHv4 07/21] (trivial) t3303: Indent with tabs instead of spaces for consistency Johan Herland
2010-10-21 2:08 ` [PATCHv4 08/21] notes.c: Use two newlines (instead of one) when concatenating notes Johan Herland
2010-10-21 2:08 ` [PATCHv4 09/21] builtin/notes.c: Split notes ref DWIMmery into a separate function Johan Herland
2010-10-21 2:08 ` [PATCHv4 10/21] git notes merge: Initial implementation handling trivial merges only Johan Herland
2010-10-21 2:08 ` [PATCHv4 11/21] builtin/notes.c: Refactor creation of notes commits Johan Herland
2010-10-21 2:08 ` [PATCHv4 12/21] git notes merge: Handle real, non-conflicting notes merges Johan Herland
2010-10-21 2:08 ` [PATCHv4 13/21] git notes merge: Add automatic conflict resolvers (ours, theirs, union) Johan Herland
2010-10-21 2:08 ` [PATCHv4 14/21] Documentation: Preliminary docs on 'git notes merge' Johan Herland
2010-10-21 2:08 ` [PATCHv4 15/21] git notes merge: Manual conflict resolution, part 1/2 Johan Herland
2010-10-21 2:08 ` [PATCHv4 16/21] git notes merge: Manual conflict resolution, part 2/2 Johan Herland
2010-10-21 2:08 ` [PATCHv4 17/21] git notes merge: List conflicting notes in notes merge commit message Johan Herland
2010-10-21 2:08 ` [PATCHv4 18/21] git notes merge: --commit should fail if underlying notes ref has moved Johan Herland
2010-10-21 2:08 ` [PATCHv4 19/21] git notes merge: Add another auto-resolving strategy: "cat_sort_uniq" Johan Herland
2010-10-21 2:08 ` [PATCHv4 20/21] git notes merge: Add testcases for merging notes trees at different fanouts Johan Herland
2010-10-21 2:08 ` [PATCHv4 21/21] Provide 'git notes get-ref' to easily retrieve current notes ref Johan Herland
2010-10-21 21:00 ` [PATCHv4 00/21] git notes merge Sverre Rabbelier
2010-10-21 23:20 ` Junio C Hamano
2010-10-21 23:30 ` Jonathan Nieder
2010-10-22 14:11 ` git merge --abort? [was: Re: [PATCHv4 00/21] git notes merge] Johan Herland
2010-10-22 14:55 ` Jonathan Nieder
2010-10-22 15:12 ` Johan Herland
2010-10-22 15:20 ` Sverre Rabbelier
2010-10-22 15:48 ` Jonathan Nieder
2010-10-22 15:57 ` Sverre Rabbelier
2010-10-22 15:41 ` [PATCHv4 00/21] git notes merge Johan Herland
2010-10-22 15:54 ` Sverre Rabbelier
2010-10-23 0:47 ` Johan Herland
2010-10-23 1:44 ` Sverre Rabbelier
2010-10-22 22:28 ` Johan Herland
2010-10-23 1:38 ` Sverre Rabbelier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1287626936-32232-7-git-send-email-johan@herland.net \
--to=johan@herland.net \
--cc=avarab@gmail.com \
--cc=bebarino@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).