Re: [PATCH v2 6/8] dir: fix checks on common prefix directory

> On Mon, Dec 16, 2019 at 4:04 PM Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Mon, 16 Dec 2019, Elijah Newren wrote:
> > > On Mon, Dec 16, 2019 at 5:51 AM Elijah Newren <newren@gmail.com> wrote:
> > > >
> > > > On Sun, Dec 15, 2019 at 2:29 AM Johannes Schindelin
> > > > <Johannes.Schindelin@gmx.de> wrote:
> > > > >
> > > > > Hi Elijah,
> > > > >
> > > > > I have not had time to dive deeply into this, but I know that it _does_
> > > > > cause a ton of segmentation faults in the `shears/pu` branch (where all of
> > > > > Git for Windows' patches are rebased on top of `pu`):
> > > >
> > > > Weird.  If it's going to cause segmentation faults at all, it would
> > > > certainly do it all over the place, but I tested the patches on the
> > > > major platforms using your Azure Pipelines setup on git.git so it
> > > > should be good on all the platforms.  Did your shears/pu branch make
> > > > some other changes to the setup?
> >
> > Not really.
> >
> > >
> > > Actually, it looks like I looked up the definition of dirent
> > > previously and forgot by the time you emailed.  On linux, from
> > > /usr/include/bits/dirent.h:
> ...
> > > and from compat/win32/dirent.h defines it as:
> > >
> > > struct dirent {
> > >         unsigned char d_type;      /* file type to prevent lstat after
> > > readdir */
> > >         char d_name[MAX_PATH * 3]; /* file name (* 3 for UTF-8 conversion) */
> > > };
> ...
> >
> > If you care to look at our very own `compat/win32/dirent.h`, you will see
> > this:
>
> Interesting, we both brought up compat/win32/dirent.h and quoted from
> it in our emails...
>
> > struct dirent {
> >         unsigned char d_type; /* file type to prevent lstat after readdir */
> >         char *d_name;         /* file name */
> > };
>
> ...but the contents were different?  Looks like git-for-windows forked
> compat/win32/dirent.h, possibly in a way that violates POSIX as
> pointed out by Junio.

> Any reason those changes weren't sent back upstream, by chance?  Feels
> odd having a compat/win32/ directory that our downstream windows users
> aren't actually using.  It also means the testing I'm getting from
> gitgitgadget and your Azure setup (which all is really, really nice by
> the way), is far less reassuring and helpful than I hoped.

> > And looking at
> > https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/dirent.h.html, I
> > do not see any guarantee of that `[256]` at all:
> >
> > The <dirent.h> header shall [...] define the structure dirent which shall
> > include the following members:
> >
> > [XSI][Option Start]
> > ino_t  d_ino       File serial number.
> > [Option End]
> > char   d_name[]    Filename string of entry.
> >
> > You will notice that not even `d_type` is guaranteed.
>
> Doh, yeah, I messed that up too.
>
> Anyway, as I mentioned to Junio, I'll resubmit after gutting the
> series.  I'll still include a fix for the issue that a real world user
> reported, but all the other ancillary bugs I found that have been
> around for over a decade aren't important enough to merit a major
> refactor, IMO.