Re: [PATCH 6/6] Forbid "the_index" in dir.c and unpack-trees.c

[Brandon Williams <bmwill@google.com>]

On 06/05, Nguyễn Thái Ngọc Duy wrote:
> Use of global variables like the_index makes it very hard to follow
> the code flow, especially when it's buried deep in some minor helper
> function.
> 
> We are gradually avoiding global variables, this hopefully will make
> it one step closer. The end game is, the set of "library" source files
> will have just one macro set: "LIB_CODE" (or something). This macro
> will prevent access to most (if not all) global variables in those
> files. We can't do that now, so we just prevent one thing at a time.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Should I keep my trick of defining the_index to
>  the_index_should_not_be_used_here? It may help somewhat when people
>  accidentally use the_index.

We already have a set of index compatibility macros which this could
maybe be a part of.  Though if we want to go this way I think we should
do the reverse of this and instead have GLOBAL_INDEX which must be
defined in order to have access to the global.  That way new files are
already protected by this and it may be easier to reduce the number of
these defines through our codebase than to add them to every single
file.

> 
>  cache.h        | 2 ++
>  dir.c          | 2 ++
>  unpack-trees.c | 2 ++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/cache.h b/cache.h
> index 89a107a7f7..ecc96ccb0e 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -330,7 +330,9 @@ struct index_state {
>  	struct ewah_bitmap *fsmonitor_dirty;
>  };
>  
> +#ifndef NO_GLOBAL_INDEX
>  extern struct index_state the_index;
> +#endif
>  
>  /* Name hashing */
>  extern int test_lazy_init_name_hash(struct index_state *istate, int try_threaded);
> diff --git a/dir.c b/dir.c
> index ccf8b4975e..74d848db5a 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -8,6 +8,8 @@
>   *		 Junio Hamano, 2005-2006
>   */
>  #define NO_THE_INDEX_COMPATIBILITY_MACROS
> +/* Do not use the_index. You should have access to it via function arg */
> +#define NO_GLOBAL_INDEX
>  #include "cache.h"
>  #include "config.h"
>  #include "dir.h"
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3ace82ca27..9aebe9762b 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1,4 +1,6 @@
>  #define NO_THE_INDEX_COMPATIBILITY_MACROS
> +/* Do not use the_index here, you probably want o->src_index */
> +#define NO_GLOBAL_INDEX
>  #include "cache.h"
>  #include "argv-array.h"
>  #include "repository.h"
> -- 
> 2.18.0.rc0.333.g22e6ee6cdf
> 

-- 
Brandon Williams