Re: [PATCH 14/40] sha1_file: prepare for external odbs

From: Jeff Hostetler <git@jeffhostetler.com>
Date: Jan 4, 2018, 18:00 UTC
Message-ID: <33af3206-b15d-3b01-e121-796cd31eb743@jeffhostetler.com>
In-Reply-To: <20180103163403.11303-15-chriscool@tuxfamily.org>

On 1/3/2018 11:33 AM, Christian Couder wrote:

Show 10 quoted lines

> In the following commits we will need some functions that were
> internal to sha1_file.c, so let's first make them non static
> and declare them in "cache.h". While at it, let's rename
> 'create_tmpfile()' to 'create_object_tmpfile()' to make its
> name less generic.
> 
> Let's also split out 'sha1_file_name_alt()' from
> 'sha1_file_name()' and 'open_sha1_file_alt()' from
> 'open_sha1_file()', as we will need both of these new
> functions too.

[...]

Show 12 quoted lines

> diff --git a/sha1_file.c b/sha1_file.c
> index 261baf800f..785e8dda03 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -322,17 +322,22 @@ static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
>   	}
>   }
>   
> -const char *sha1_file_name(const unsigned char *sha1)
> +const char *sha1_file_name_alt(const char *objdir, const unsigned char *sha1)
>   {
>   	static struct strbuf buf = STRBUF_INIT;

While we are refactoring sha1_file_name() and adding sha1_file_name_alt(), could we also change the API and pass in the strbuf so we can get rid of the static buffer? Granted, it is a little off topic, but it will help out in the long run.

[...]

Show 11 quoted lines

> @@ -1551,7 +1562,7 @@ static inline int directory_size(const char *filename)
>    * We want to avoid cross-directory filename renames, because those
>    * can have problems on various filesystems (FAT, NFS, Coda).
>    */
> -static int create_tmpfile(struct strbuf *tmp, const char *filename)
> +int create_object_tmpfile(struct strbuf *tmp, const char *filename)
>   {
>   	int fd, dirlen = directory_size(filename);
>   
> @@ -1591,7 +1602,7 @@ static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
>   	static struct strbuf tmp_file = STRBUF_INIT;

Same thing here, since we are renaming the function anyway, could we add a strbuf arg and get rid of the static one?

Show 8 quoted lines

>   	const char *filename = sha1_file_name(sha1);
>   
> -	fd = create_tmpfile(&tmp_file, filename);
> +	fd = create_object_tmpfile(&tmp_file, filename);
>   	if (fd < 0) {
>   		if (errno == EACCES)
>   			return error("insufficient permission for adding an object to repository database %s", get_object_directory());
>

Jeff