Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag

[René Scharfe]

Am 10.04.2018 um 00:45 schrieb Stefan Beller:
> By making the oidmap a pointer, we eliminate the need for
> the global boolean variable 'replace_object_prepared'.
> 
> Signed-off-by: Stefan Beller 
> ---
>   object-store.h   |  2 +-
>   replace-object.c | 16 +---
>   2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/object-store.h b/object-store.h
> index c04b4c95eb..1ff862c7f9 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -99,7 +99,7 @@ struct raw_object_store {
>* Objects that should be substituted by other objects
>* (see git-replace(1)).
>*/
> - struct oidmap replace_map;
> + struct oidmap *replace_map;

This also allows the '#include "oidmap.h"' introduced in patch 3 to be
replaced by 'struct oidmap;' (forward declaration instead of include).
Keeping the type opaque discourages circumventing accessor functions;
not dragging in other headers avoids some compile time overhead.

René

Re: [PATCH 05/16] replace-object: eliminate replace objects prepared flag

[Junio C Hamano]

Stefan Beller  writes:

> By making the oidmap a pointer, we eliminate the need for
> the global boolean variable 'replace_object_prepared'.

That is not quite a justification for this change, as making it a
pointer (and paying for the malloc(3) overhead) is not the only way
to remove the variable (i.e. the "has this been initialized?" bit
can be moved to "struct raw_object_store").

One possible advantage of this approach, I guess, is that we would
more quickly catch code that tries to access replace-map without
initializing it.

[PATCH 05/16] replace-object: eliminate replace objects prepared flag

[Stefan Beller]

By making the oidmap a pointer, we eliminate the need for
the global boolean variable 'replace_object_prepared'.

Signed-off-by: Stefan Beller 
---
 object-store.h   |  2 +-
 replace-object.c | 16 +---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/object-store.h b/object-store.h
index c04b4c95eb..1ff862c7f9 100644
--- a/object-store.h
+++ b/object-store.h
@@ -99,7 +99,7 @@ struct raw_object_store {
 * Objects that should be substituted by other objects
 * (see git-replace(1)).
 */
-   struct oidmap replace_map;
+   struct oidmap *replace_map;
 
/*
 * private data
diff --git a/replace-object.c b/replace-object.c
index afbdf2df25..953fa9cc40 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -25,7 +25,7 @@ static int register_replace_ref(const char *refname,
oidcpy(_obj->replacement, oid);
 
/* Register new object */
-   if (oidmap_put(_repository->objects->replace_map, repl_obj))
+   if (oidmap_put(the_repository->objects->replace_map, repl_obj))
die("duplicate replace ref: %s", refname);
 
return 0;
@@ -33,14 +33,16 @@ static int register_replace_ref(const char *refname,
 
 static void prepare_replace_object(void)
 {
-   static int replace_object_prepared;
-
-   if (replace_object_prepared)
+   if (the_repository->objects->replace_map)
return;
 
+   the_repository->objects->replace_map =
+   xmalloc(sizeof(*the_repository->objects->replace_map));
+   oidmap_init(the_repository->objects->replace_map, 0);
+
for_each_replace_ref(register_replace_ref, NULL);
-   replace_object_prepared = 1;
-   if (!the_repository->objects->replace_map.map.tablesize)
+
+   if (!the_repository->objects->replace_map->map.tablesize)
check_replace_refs = 0;
 }
 
@@ -64,7 +66,7 @@ const struct object_id *do_lookup_replace_object(const struct 
object_id *oid)
/* Try to recursively replace the object */
while (depth-- > 0) {
struct replace_object *repl_obj =
-   oidmap_get(_repository->objects->replace_map, cur);
+   oidmap_get(the_repository->objects->replace_map, cur);
if (!repl_obj)
return cur;
cur = _obj->replacement;
-- 
2.17.0.484.g0c8726318c-goog