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

> Johan Herland wrote:
> > 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
> 
> Would it makes sense to split off that no-op as a separate patch?

> > --- a/notes.c
> > +++ b/notes.c
> > @@ -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) {
> 
> No note present, but the node for one is.  This skips insertion of
> empty notes, for consistency with:
> 
> > @@ -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);
> 
> The note-present case, where the combine_notes() function can
> return a null sha1 to request that a note be removed.
> 
> >  				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;
> > +	}
> 
> The more usual no-note-present case.  Again, this skips insertion
> of empty notes.