From: Johan Herland <johan@herland.net>
Date: Thu, 21 Oct 2010 13:16:22 GMT
Subject: Re: [PATCHv4 06/21] notes.h/c: Propagate combine_notes_fn return value to add_note() and beyond
Message-ID: <201010211516.23091.johan@herland.net>
In-Reply-To: <20101021052100.GB2413@burratino>

On Thursday 21 October 2010, Jonathan Nieder wrote:
> Johan Herland wrote:
> > --- a/notes.c
> > +++ b/notes.c
> > @@ -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;
>
> Micronit: it would probably be clearer to write
>
> 	if (ret)
> 		return ret;
> 	*p = SET_PTR_TYPE(...
> 	return note_tree_insert(...
>
> to avoid a little nesting.

Agreed. Will be fixed in the next iteration.


Thanks,

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net