Re: [PATCH 1/5] fetch-pack: sort incoming heads

[Jeff King <peff@peff.net>]

On Tue, May 22, 2012 at 01:08:42PM -0700, Junio C Hamano wrote:

> > @@ -1076,6 +1081,8 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
> >  			st.st_mtime = 0;
> >  	}
> >  
> > +	qsort(heads, nr_heads, sizeof(*heads), compare_heads);
> > +
> >  	if (heads && nr_heads)
> >  		nr_heads = remove_duplicates(nr_heads, heads);
> 
> Hrm, could heads and/or nr_heads be NULL/0 here when we try to run qsort()
> in this codepath?

Good catch. I had originally put the qsort into remove_duplicates, but
hoisted it out, as the second optimization depends on the sorting, too.
heads can be NULL here (for example, if you run fetch-pack without any
arguments, and without --stdin; though why you would do so is a
mystery, we should protect against it).

A sane qsort would see that its second parameter is 0 and never try to
dereference the array. But I'm not sure all qsort implementations we
will see are sane, so it's probably better to protect it by putting it
inside the conditional block just below.

-Peff