From: Derrick Stolee <stolee@gmail.com>
Date: Mon, 09 Apr 2018 13:51:39 GMT
Subject: Re: [PATCH 02/19] replace-object: move replace_object to object store
Message-ID: <9cf5ca7b-4837-8626-f4f6-f580fd33128b@gmail.com>
In-Reply-To: <20180406232136.253950-3-sbeller@google.com>

On 4/6/2018 7:21 PM, Stefan Beller wrote:
> Refs belong to particular repositories, so the replacements defined by
> them should belong to a particular repository as well.
>
> Move the definition of a single object replacement to a new header
> "replace-object.h". While at it replace the hardcoded 20 by GIT_MAX_RAWSZ.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>   object-store.h   | 14 ++++++++++++++
>   replace-object.c | 40 ++++++++++++++++++----------------------
>   replace-object.h |  9 +++++++++
>   3 files changed, 41 insertions(+), 22 deletions(-)
>   create mode 100644 replace-object.h

Throughout this commit, there appears to be an extra space inserted 
before 'the_repository'. Some are more obvious than others (such as a 
'free( the_repository->...)' but others are after the indentation.

> diff --git a/object-store.h b/object-store.h
> index fef33f345f..da639b3184 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -93,6 +93,20 @@ struct raw_object_store {
>   	struct alternate_object_database *alt_odb_list;
>   	struct alternate_object_database **alt_odb_tail;
>   
> +	/*
> +	 * Objects that should be substituted by other objects
> +	 * (see git-replace(1)).
> +	 */
> +	struct replace_objects {
> +		/*
> +		 * An array of replacements.  The array is kept sorted by the original
> +		 * sha1.
> +		 */
> +		struct replace_object **items;
> +
> +		int alloc, nr;
> +	} replacements;
> +
>   	/*
>   	 * private data
>   	 *
> diff --git a/replace-object.c b/replace-object.c
> index 3e49965d05..a7eb31026e 100644
> --- a/replace-object.c
> +++ b/replace-object.c
> @@ -1,19 +1,11 @@
>   #include "cache.h"
> +#include "replace-object.h"
> +#include "object-store.h"
>   #include "sha1-lookup.h"
>   #include "refs.h"
> +#include "repository.h"
>   #include "commit.h"
>   
> -/*
> - * An array of replacements.  The array is kept sorted by the original
> - * sha1.
> - */
> -static struct replace_object {
> -	unsigned char original[20];
> -	unsigned char replacement[20];
> -} **replace_object;
> -
> -static int replace_object_alloc, replace_object_nr;
> -
>   static const unsigned char *replace_sha1_access(size_t index, void *table)
>   {
>   	struct replace_object **replace = table;
> @@ -22,7 +14,8 @@ static const unsigned char *replace_sha1_access(size_t index, void *table)
>   
>   static int replace_object_pos(const unsigned char *sha1)
>   {
> -	return sha1_pos(sha1, replace_object, replace_object_nr,
> +	return sha1_pos(sha1,  the_repository->objects->replacements.items,
> +			 the_repository->objects->replacements.nr,
>   			replace_sha1_access);
>   }
>   
> @@ -35,18 +28,21 @@ static int register_replace_object(struct replace_object *replace,
>   		if (ignore_dups)
>   			free(replace);
>   		else {
> -			free(replace_object[pos]);
> -			replace_object[pos] = replace;
> +			free( the_repository->objects->replacements.items[pos]);
> +			 the_repository->objects->replacements.items[pos] = replace;
>   		}
>   		return 1;
>   	}
>   	pos = -pos - 1;
> -	ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);
> -	replace_object_nr++;
> -	if (pos < replace_object_nr)
> -		MOVE_ARRAY(replace_object + pos + 1, replace_object + pos,
> -			   replace_object_nr - pos - 1);
> -	replace_object[pos] = replace;
> +	ALLOC_GROW( the_repository->objects->replacements.items,
> +		    the_repository->objects->replacements.nr + 1,
> +		    the_repository->objects->replacements.alloc);
> +	 the_repository->objects->replacements.nr++;
> +	if (pos <  the_repository->objects->replacements.nr)
> +		MOVE_ARRAY( the_repository->objects->replacements.items + pos + 1,
> +			    the_repository->objects->replacements.items + pos,
> +			    the_repository->objects->replacements.nr - pos - 1);
> +	 the_repository->objects->replacements.items[pos] = replace;
>   	return 0;
>   }
>   
> @@ -84,7 +80,7 @@ static void prepare_replace_object(void)
>   
>   	for_each_replace_ref(register_replace_ref, NULL);
>   	replace_object_prepared = 1;
> -	if (!replace_object_nr)
> +	if (!the_repository->objects->replacements.nr)
>   		check_replace_refs = 0;
>   }
>   
> @@ -113,7 +109,7 @@ const unsigned char *do_lookup_replace_object(const unsigned char *sha1)
>   
>   		pos = replace_object_pos(cur);
>   		if (0 <= pos)
> -			cur = replace_object[pos]->replacement;
> +			cur = the_repository->objects->replacements.items[pos]->replacement;
>   	} while (0 <= pos);
>   
>   	return cur;
> diff --git a/replace-object.h b/replace-object.h
> new file mode 100644
> index 0000000000..50731ec9c2
> --- /dev/null
> +++ b/replace-object.h
> @@ -0,0 +1,9 @@
> +#ifndef REPLACE_OBJECT_H
> +#define REPLACE_OBJECT_H
> +
> +struct replace_object {
> +	unsigned char original[GIT_MAX_RAWSZ];
> +	unsigned char replacement[GIT_MAX_RAWSZ];
> +};
> +
> +#endif /* REPLACE_OBJECT_H */