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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 08.01.22 um 09:54 schrieb Han Xin:
> From: Han Xin <hanxin.hx@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Han Xin <hanxin.hx@xxxxxxxxxxxxxxx>
> ---
>  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?

>  			break;
>  		if (adjust_shared_perm(tmp->buf))




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux