Re: [PATCH 2/2] fetch/push: readd rsync support
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Sep 28, 2007, 12:53 UTC
- Message-ID
- <Pine.LNX.4.64.0709281350140.28395@racer.site>
- In-Reply-To
- <7v3awzqgqc.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 28 Sep 2007, Junio C Hamano wrote:
Show 51 quoted lines
> 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.
Show 6 quoted lines
> > ... > > + 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