Re: [PATCH v5 2/3] Refactor submodule push check to use string list instead of integer

From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Mar 26, 2012, 19:55 UTC
Message-ID: <20120326195535.GA41511@book.hvoigt.net>
In-Reply-To: <20120326193317.GB41087@book.hvoigt.net>

On Mon, Mar 26, 2012 at 09:33:17PM +0200, Heiko Voigt wrote:

Show 16 quoted lines

> On Mon, Feb 13, 2012 at 07:28:24PM -0800, Junio C Hamano wrote:
> > Heiko Voigt <hvoigt@hvoigt.net> writes:
> > > -			*needs_pushing = 1;
> > > -			break;
> > > +			if (!string_list_has_string(needs_pushing, p->two->path))
> > > +				string_list_insert(needs_pushing, p->two->path);
> > 
> > Does string_list API have "look for this and insert if it doesn't exist
> > but otherwise don't do anything"?  Running get_entry_index() to answer
> > has_string() once and then calling it again to find where to insert to
> > respond to insert() looks a bit wasteful.
> > 
> > Just wondering.
> 
> If I see correctly currently it has no such functionality. I can have a
> look at adding another patch implementing this.

It seems my guess was wrong. As far as I read in the code string_list_insert() already skips inserting existing strings and just returns the existing items in the list. So it should be fine to remove the string_list_has_string() call.

Cheers Heiko