Re: [PATCH v8 3/6] object-file.c: remove the slash for directory_size()

>
> Am 08.01.22 um 09:54 schrieb Han Xin:
> > From: Han Xin <hanxin.hx@alibaba-inc.com>
> >
> > Since "mkdir foo/" works as well as "mkdir foo", let's remove the end
> > slash as many users of it want.
> >
> > Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> > Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com>
> > ---
> >  object-file.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/object-file.c b/object-file.c
> > index 5d163081b1..4f0127e823 100644
> > --- a/object-file.c
> > +++ b/object-file.c
> > @@ -1831,13 +1831,13 @@ static void close_loose_object(int fd)
> >               die_errno(_("error when closing loose object file"));
> >  }
> >
> > -/* Size of directory component, including the ending '/' */
> > +/* Size of directory component, excluding the ending '/' */
> >  static inline int directory_size(const char *filename)
> >  {
> >       const char *s = strrchr(filename, '/');
> >       if (!s)
> >               return 0;
> > -     return s - filename + 1;
> > +     return s - filename;
>
> This will return zero both for "filename" and "/filename".  Hmm.  Since
> it's only used for loose object files we can assume that at least one
> slash is present, so this removal of functionality is not actually a
> problem.  But I don't understand its benefit.
>
> >  }
> >
> >  /*
> > @@ -1854,7 +1854,7 @@ static int create_tmpfile(struct strbuf *tmp, const char *filename,
> >
> >       strbuf_reset(tmp);
> >       strbuf_add(tmp, filename, dirlen);
> > -     strbuf_addstr(tmp, "tmp_obj_XXXXXX");
> > +     strbuf_addstr(tmp, "/tmp_obj_XXXXXX");
> >       fd = git_mkstemp_mode(tmp->buf, 0444);
> >       do {
> >               if (fd >= 0 || !dirlen || errno != ENOENT)
> > @@ -1866,7 +1866,7 @@ static int create_tmpfile(struct strbuf *tmp, const char *filename,
> >                * scratch.
> >                */
> >               strbuf_reset(tmp);
> > -             strbuf_add(tmp, filename, dirlen - 1);
> > +             strbuf_add(tmp, filename, dirlen);
> >               if (mkdir(tmp->buf, 0777) && errno != EEXIST)
>
> This code makes sure that mkdir(2) is called without the trailing slash,
> both with or without this patch.  From the commit message above I
> somehow expected a change in this regard -- but again I wouldn't
> understand its benefit.
>
> Is this change really needed?  Is streaming unpack not possible with the
> original directory_size() function?
>