Re: [PATCH v8 3/6] object-file.c: remove the slash for directory_size()
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 8, 2022, 17:24 UTC
- Message-ID
- <6d63d5d2-48db-40e9-8e5c-5b72c3d84414@web.de>
- In-Reply-To
- <20220108085419.79682-4-chiyutianyi@gmail.com>
Am 08.01.22 um 09:54 schrieb Han Xin:
Show 28 quoted lines
> 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.
Show 19 quoted lines
> }
>
> /*
> @@ -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?
> break; > if (adjust_shared_perm(tmp->buf))