On Saturday 12 September 2009, Junio C Hamano wrote:
> Johan Herland <johan@xxxxxxxxxxx> writes:
> > This is a major expansion of the notes lookup code to allow for
> > variations in the notes tree organization. The variations allowed
> > include mixing fanout schemes based on the commit dates of the
> > annotated commits (aka. date-based fanout) with fanout schemes based on
> > the SHA1 of the annotated commits (aka. SHA1-based fanout).
Note that this patch is about to be removed from this series, cf. today's
discussion with Shawn elsewhere in this thread.
> Will squash this in.
I agree with the attempt, but not all of the hunks are good:
> @@ -281,20 +281,20 @@ static int note_tree_insert(struct int_node *tree,
> unsigned char n, /* Free the entire notes data contained in the given
> tree */
> static void note_tree_free(struct int_node *tree)
> {
> - if (tree->magic == (void *) ~0) {
> - if (tree->prev) {
> - note_tree_free(tree->prev);
> - free(tree->prev);
> + if (tree->u.s.magic == (void *) ~0) {
> + if (tree->u.s.prev) {
> + note_tree_free(tree->u.s.prev);
> + free(tree->u.s.prev);
> }
> - if (tree->child) {
> - note_tree_free(tree->child);
> - free(tree->child);
> + if (tree->u.s.magic) {
> + note_tree_free(tree->u.s.magic);
> + free(tree->u.s.magic);
Here, you are replacing tree->child with tree->u.s.magic. Shouldn't that be
tree->u.s.child instead?
> @@ -439,12 +439,12 @@ static void load_date_subtree(struct tree_desc
> *tree_desc, else /* this is the last entry, store directly into node */
> new_node = node;
>
> - new_node->magic = (void *) ~0;
> - new_node->child = NULL;
> - new_node->prev = cur_node;
> - new_node->parent = parent;
> - hashcpy(new_node->tree_sha1, entry.sha1);
> - strcpy(new_node->period, period);
> + new_node->u.s.magic = (void *) ~0;
> + new_node->u.s.magic = NULL;
Same as above: new_node->u.s.child
> + new_node->u.s.prev = cur_node;
> + new_node->u.s.parent = parent;
> + hashcpy(new_node->u.s.tree_sha1, entry.sha1);
> + strcpy(new_node->u.s.period, period);
> cur_node = new_node;
> }
> assert(!cur_node || cur_node == node);
> @@ -552,38 +552,38 @@ static unsigned char *lookup_notes(const struct
> commit *commit) /* Convert commit->date to YYYY-MM-DD format */
> short_date = show_date(commit->date, 0, DATE_SHORT);
>
> - while (node->magic == (void *) ~0) { /* date-based node */
> - int cmp = SUBTREE_DATE_PREFIXCMP(short_date, node->period);
> + while (node->u.s.magic == (void *) ~0) { /* date-based node */
> + int cmp = SUBTREE_DATE_PREFIXCMP(short_date, node->u.s.period);
> if (cmp == 0) {
> /* Search inside child node */
> - if (!node->child) {
> + if (!node->u.s.magic) {
> /* Must unpack child node first */
> - node->child = (struct int_node *)
> + node->u.s.magic = (struct int_node *)
> xcalloc(sizeof(struct int_node), 1);
> - load_subtree(node->tree_sha1,
> - (const unsigned char *) node->period,
> - strlen(node->period), node->child,
> + load_subtree(node->u.s.tree_sha1,
> + (const unsigned char *) node->u.s.period,
> + strlen(node->u.s.period), node->u.s.magic,
> node, -1);
> }
> seen_node = node;
> - node = node->child;
> + node = node->u.s.magic;
Same again, 4 times in the above hunk.
> @@ -591,15 +591,15 @@ static unsigned char *lookup_notes(const struct
> commit *commit) }
> }
> while (cur_node &&
> - SUBTREE_DATE_PREFIXCMP(cur_node->period, seen_node->period) < 0)
> + SUBTREE_DATE_PREFIXCMP(cur_node->u.s.period,
> seen_node->u.s.period) < 0) {
> /*
> * We're about to move cur_node backwards in history. We are
> * unlikely to need this cur_node in the future, so free() it.
> */
> - note_tree_free(cur_node->child);
> - cur_node->child = NULL;
> - cur_node = cur_node->parent;
> + note_tree_free(cur_node->u.s.magic);
> + cur_node->u.s.magic = NULL;
...and another one here.
> + cur_node = cur_node->u.s.parent;
> }
> cur_node = seen_node;
>
But as I said above, you may want to drop 13/14 and 14/14 completely,
instead.
...Johan
--
Johan Herland, <johan@xxxxxxxxxxx>
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html