Re: [PATCH 2/2] fetch/push: readd rsync support

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Fri, 28 Sep 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > +/*
> > + * path is assumed to point to a buffer of PATH_MAX bytes, and
> > + * path + name_offset is expected to point to "refs/".
> > + */
> > +
> > +static int read_loose_refs(char *path, int name_offset, struct ref **tail)
> > +{
> > +	DIR *dir = opendir(path);
> > +	struct dirent *de;
> > +	struct {
> > +		struct dirent *entries;
> > +		int nr, alloc;
> > +	} list;
> > +	int i, pathlen;
> > +
> > +	if (!dir)
> > +		return -1;
> > +
> > +	memset (&list, 0, sizeof(list));
> > +
> > +	while ((de = readdir(dir))) {
> > +		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> > +				(de->d_name[1] == '.' &&
> > +				 de->d_name[2] == '\0')))
> > +			continue;
> > +		if (list.nr >= list.alloc) {
> > +			list.alloc = alloc_nr(list.nr);
> > +			list.entries = xrealloc(list.entries,
> > +				list.alloc * sizeof(*de));
> > +		}
> 
> ALLOC_GROW() not applicable here?
> 
> > +		list.entries[list.nr++] = *de;
> 
> Are you sure about this?
> 
> The last paragraph in Rationale section, in
> 
>     http://www.opengroup.org/onlinepubs/000095399/basedefs/dirent.h.html
> 
> suggests that the d_name[] member in struct dirent could be
> declared at the very end of the structure as length of 1 (the
> traditional trick to implement a flex-array); your assignment
> >from *de into entries[] would not work as expected on such an
> implementation.
> 
> On Linux with glibc it appears bits/dirent.h defines dirent with
> "char d_name[256]", so you may not see a breakage there, though.

D'oh!  You're completely right.

> You only use a list of strings (char **), don't you?

Originally, I wanted to use the d_type, too, but as I mentioned on IRC, it 
is not reliable enough (shows DT_UNKNOWN _all_ the time here).

But yes, I'll redo it, using ALLOC_GROW.

> > ...
> > +			if (fd < 0)
> > +				continue;
> > +			next = alloc_ref(strlen(path + name_offset));
> 
> And as we discussed earlier you would need one more byte here ;-).

Well, not after my patch, which I thought of as (1/3), but then decided it 
is good enough to be (1/1) until you shot it down...

Will fix.

Ciao,
Dscho