git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 03/18] notes.h/c: Clarify the handling of notes objects that are == null_sha1

From
Johan Herland <johan@herland.net>
Date
Sep 29, 2010, 00:23 UTC
Message-ID
<1285719811-10871-4-git-send-email-johan@herland.net>
In-Reply-To
<1285719811-10871-1-git-send-email-johan@herland.net>

Clearly specify how combine_notes functions are expected to handle null_sha1 in input, and how they may set the result to null_sha1 in order to cause the removal of a note.

Also document that passing note_sha1 == null_sha1 to add_note() is usually a no-op, except in cases where combining it with an existing note yields a new/changed result.

The only functional changes in this patch concern the handling of null_sha1 in notes_tree_insert(). Otherwise the patch consists solely of reordering functions in notes.c to avoid use-before-declaration, and adding documentation to notes.h.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  158 +++++++++++++++++++++++++++++++++-----------------------------
 notes.h |   16 ++++++-
 2 files changed, 99 insertions(+), 75 deletions(-)
diff --git a/notes.c b/notes.c
index 79fd850..9e92021 100644
--- a/notes.c
+++ b/notes.c
@@ -150,6 +150,79 @@ static struct leaf_node *note_tree_find(struct notes_tree *t,
 }
 
 /*
+ * How to consolidate an int_node:
+ * If there are > 1 non-NULL entries, give up and return non-zero.
+ * Otherwise replace the int_node at the given index in the given parent node
+ * with the only entry (or a NULL entry if no entries) from the given tree,
+ * and return 0.
+ */
+static int note_tree_consolidate(struct int_node *tree,
+	struct int_node *parent, unsigned char index)
+{
+	unsigned int i;
+	void *p = NULL;
+
+	assert(tree && parent);
+	assert(CLR_PTR_TYPE(parent->a[index]) == tree);
+
+	for (i = 0; i < 16; i++) {
+		if (GET_PTR_TYPE(tree->a[i]) != PTR_TYPE_NULL) {
+			if (p) /* more than one entry */
+				return -2;
+			p = tree->a[i];
+		}
+	}
+
+	/* replace tree with p in parent[index] */
+	parent->a[index] = p;
+	free(tree);
+	return 0;
+}
+
+/*
+ * To remove a leaf_node:
+ * Search to the tree location appropriate for the given leaf_node's key:
+ * - If location does not hold a matching entry, abort and do nothing.
+ * - Replace the matching leaf_node with a NULL entry (and free the leaf_node).
+ * - Consolidate int_nodes repeatedly, while walking up the tree towards root.
+ */
+static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
+		unsigned char n, struct leaf_node *entry)
+{
+	struct leaf_node *l;
+	struct int_node *parent_stack[20];
+	unsigned char i, j;
+	void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
+
+	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
+	if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
+		return; /* type mismatch, nothing to remove */
+	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
+	if (hashcmp(l->key_sha1, entry->key_sha1))
+		return; /* key mismatch, nothing to remove */
+
+	/* we have found a matching entry */
+	free(l);
+	*p = SET_PTR_TYPE(NULL, PTR_TYPE_NULL);
+
+	/* consolidate this tree level, and parent levels, if possible */
+	if (!n)
+		return; /* cannot consolidate top level */
+	/* first, build stack of ancestors between root and current node */
+	parent_stack[0] = t->root;
+	for (i = 0; i < n; i++) {
+		j = GET_NIBBLE(i, entry->key_sha1);
+		parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
+	}
+	assert(i == n && parent_stack[i] == tree);
+	/* next, unwind stack until note_tree_consolidate() is done */
+	while (i > 0 &&
+	       !note_tree_consolidate(parent_stack[i], parent_stack[i - 1],
+				      GET_NIBBLE(i - 1, entry->key_sha1)))
+		i--;
+}
+
+/*
  * To insert a leaf_node:
  * Search to the tree location appropriate for the given leaf_node's key:
  * - If location is unused (NULL), store the tweaked pointer directly there
@@ -175,7 +248,10 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
 	switch (GET_PTR_TYPE(*p)) {
 	case PTR_TYPE_NULL:
 		assert(!*p);
-		*p = SET_PTR_TYPE(entry, type);
+		if (is_null_sha1(entry->val_sha1))
+			free(entry);
+		else
+			*p = SET_PTR_TYPE(entry, type);
 		return;
 	case PTR_TYPE_NOTE:
 		switch (type) {
@@ -191,6 +267,9 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
 					    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))
+					note_tree_remove(t, tree, n, entry);
 				free(entry);
 				return;
 			}
@@ -222,6 +301,10 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
 	/* non-matching leaf_node */
 	assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
 	       GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
+	if (is_null_sha1(entry->val_sha1)) { /* skip insertion of empty note */
+		free(entry);
+		return;
+	}
 	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);
@@ -229,79 +312,6 @@ static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
 	note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
 }
 
-/*
- * How to consolidate an int_node:
- * If there are > 1 non-NULL entries, give up and return non-zero.
- * Otherwise replace the int_node at the given index in the given parent node
- * with the only entry (or a NULL entry if no entries) from the given tree,
- * and return 0.
- */
-static int note_tree_consolidate(struct int_node *tree,
-	struct int_node *parent, unsigned char index)
-{
-	unsigned int i;
-	void *p = NULL;
-
-	assert(tree && parent);
-	assert(CLR_PTR_TYPE(parent->a[index]) == tree);
-
-	for (i = 0; i < 16; i++) {
-		if (GET_PTR_TYPE(tree->a[i]) != PTR_TYPE_NULL) {
-			if (p) /* more than one entry */
-				return -2;
-			p = tree->a[i];
-		}
-	}
-
-	/* replace tree with p in parent[index] */
-	parent->a[index] = p;
-	free(tree);
-	return 0;
-}
-
-/*
- * To remove a leaf_node:
- * Search to the tree location appropriate for the given leaf_node's key:
- * - If location does not hold a matching entry, abort and do nothing.
- * - Replace the matching leaf_node with a NULL entry (and free the leaf_node).
- * - Consolidate int_nodes repeatedly, while walking up the tree towards root.
- */
-static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
-		unsigned char n, struct leaf_node *entry)
-{
-	struct leaf_node *l;
-	struct int_node *parent_stack[20];
-	unsigned char i, j;
-	void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
-
-	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
-	if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
-		return; /* type mismatch, nothing to remove */
-	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
-	if (hashcmp(l->key_sha1, entry->key_sha1))
-		return; /* key mismatch, nothing to remove */
-
-	/* we have found a matching entry */
-	free(l);
-	*p = SET_PTR_TYPE(NULL, PTR_TYPE_NULL);
-
-	/* consolidate this tree level, and parent levels, if possible */
-	if (!n)
-		return; /* cannot consolidate top level */
-	/* first, build stack of ancestors between root and current node */
-	parent_stack[0] = t->root;
-	for (i = 0; i < n; i++) {
-		j = GET_NIBBLE(i, entry->key_sha1);
-		parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
-	}
-	assert(i == n && parent_stack[i] == tree);
-	/* next, unwind stack until note_tree_consolidate() is done */
-	while (i > 0 &&
-	       !note_tree_consolidate(parent_stack[i], parent_stack[i - 1],
-				      GET_NIBBLE(i - 1, entry->key_sha1)))
-		i--;
-}
-
 /* Free the entire notes data contained in the given tree */
 static void note_tree_free(struct int_node *tree)
 {
diff --git a/notes.h b/notes.h
index 20db42f..79ea797 100644
--- a/notes.h
+++ b/notes.h
@@ -12,7 +12,10 @@
  * resulting SHA1 into the first SHA1 argument (cur_sha1). A non-zero return
  * value indicates failure.
  *
- * The two given SHA1s must both be non-NULL and different from each other.
+ * The two given SHA1s shall both be non-NULL and different from each other.
+ * Either of them (but not both) may be == null_sha1, which indicates an
+ * empty/non-existent note. If the resulting SHA1 (cur_sha1) is == null_sha1,
+ * the note will be removed from the notes tree.
  *
  * The default combine_notes function (you get this when passing NULL) is
  * combine_notes_concatenate(), which appends the contents of the new note to
@@ -90,6 +93,17 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 /*
  * Add the given note object to the given notes_tree structure
  *
+ * If there already exists a note for the given object_sha1, the given
+ * combine_notes function is invoked to break the tie. If not given (i.e.
+ * combine_notes == NULL), the default combine_notes function for the given
+ * notes_tree is used.
+ *
+ * Passing note_sha1 == null_sha1 indicates the addition of an
+ * empty/non-existent note. This is a (potentially expensive) no-op unless
+ * there already exists a note for the given object_sha1, AND combining that
+ * note with the empty note (using the given combine_notes function) results
+ * in a new/changed note.
+ *
  * 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.
-- 
1.7.3.98.g5ad7d9
Previous: Johan HerlandNext: Jonathan Nieder
Message 7 of 46 in “git notes merge”
  1. 00/18 git notes mergeJohan Herland, Sep 29, 2010
  2. 01/18 (trivial) notes.h: Minor documentation fixes to copy_notes()Johan Herland, Sep 29, 2010
  3. Jonathan NiederOct 5, 2010
  4. Johan HerlandOct 5, 2010
  5. Jonathan NiederOct 5, 2010
  6. 02/18 notes.h: Make default_notes_ref() available in notes APIJohan Herland, Sep 29, 2010
  7. 03/18 notes.h/c: Clarify the handling of notes objects that are == null_sha1Johan Herland, Sep 29, 2010
  8. Jonathan NiederOct 5, 2010
  9. Johan HerlandOct 5, 2010
  10. 04/18 notes.h/c: Propagate combine_notes_fn return value to add_note() and beyondJohan Herland, Sep 29, 2010
  11. Jonathan NiederOct 5, 2010
  12. Johan HerlandOct 6, 2010
  13. 05/18 (trivial) t3303: Indent with tabs instead of spaces for consistencyJohan Herland, Sep 29, 2010
  14. 06/18 notes.c: Use two newlines (instead of one) when concatenating notesJohan Herland, Sep 29, 2010
  15. 07/18 builtin/notes.c: Split notes ref DWIMmery into a separate functionJohan Herland, Sep 29, 2010
  16. Jonathan NiederOct 5, 2010
  17. Johan HerlandOct 7, 2010
  18. 08/18 git notes merge: Initial implementation handling trivial merges onlyJohan Herland, Sep 29, 2010
  19. Test script style (Re: [PATCH 08/18] git notes merge: Initial implementation handling trivial merges only)Jonathan Nieder, Oct 7, 2010
  20. Junio C HamanoOct 7, 2010
  21. Jonathan NiederOct 7, 2010
  22. Johan HerlandOct 8, 2010
  23. Jonathan NiederOct 9, 2010
  24. Johan HerlandOct 21, 2010
  25. 09/18 builtin/notes.c: Refactor creation of notes commits.Johan Herland, Sep 29, 2010
  26. 10/18 git notes merge: Handle real, non-conflicting notes mergesJohan Herland, Sep 29, 2010
  27. Ævar Arnfjörð BjarmasonSep 29, 2010
  28. Johan HerlandSep 29, 2010
  29. 11/18 git notes merge: Add automatic conflict resolvers (ours, theirs, union)Johan Herland, Sep 29, 2010
  30. Stephen BoydOct 2, 2010
  31. Johan HerlandOct 4, 2010
  32. 12/18 Documentation: Preliminary docs on 'git notes merge'Johan Herland, Sep 29, 2010
  33. Stephen BoydOct 2, 2010
  34. Johan HerlandOct 4, 2010
  35. 13/18 git notes merge: Manual conflict resolution, part 1/2Johan Herland, Sep 29, 2010
  36. 14/18 git notes merge: Manual conflict resolution, part 2/2Johan Herland, Sep 29, 2010
  37. Ævar Arnfjörð BjarmasonSep 29, 2010
  38. Johan HerlandSep 29, 2010
  39. 15/18 git notes merge: List conflicting notes in notes merge commit messageJohan Herland, Sep 29, 2010
  40. 16/18 git notes merge: --commit should fail if underlying notes ref has movedJohan Herland, Sep 29, 2010
  41. 17/18 git notes merge: Add another auto-resolving strategy: "cat_sort_uniq"Johan Herland, Sep 29, 2010
  42. 18/18 git notes merge: Add testcases for merging notes trees at different fanoutsJohan Herland, Sep 29, 2010
  43. Sverre RabbelierSep 29, 2010
  44. Johan HerlandSep 29, 2010
  45. Sverre RabbelierSep 29, 2010
  46. Johan HerlandSep 29, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.