threads / patch / 52544

patchsparse-checkout: use extern for global variables

Subject: [PATCH 0/1] sparse-checkout: use extern for global variables

## tl;dr

3 messages between Dec 31, 2019 and Jan 2, 2020. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Derrick Stolee via GitGitGadget· Dec 31, 2019, 13:17 UTC · lore

I noticed this issue when resolving conflicts with our VFS for Git-enabled branch in microsoft/git.

When I moved the global for core.sparseCheckout along with creating the global for core.sparseCheckoutCone, I dropped the "extern" by habit. (We are dropping these from function declarations, usually.) However, this means something different for variables, and could lead to bugs. I haven't found any, but it's better to be safe, right?

Thanks, -Stolee

Derrick Stolee (1):
  sparse-checkout: use extern for global variables

 cache.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

base-commit: 99c33bed562b41de6ce9bd3fd561303d39645048
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-506%2Fderrickstolee%2Fsparse-extern-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-506/derrickstolee/sparse-extern-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/506

-- 
gitgitgadget

Derrick Stolee via GitGitGadget· Dec 31, 2019, 13:17 UTC · re: Derrick Stolee via GitGitGadget · lore

[PATCH 1/1] sparse-checkout: use extern for global variables

From: Derrick Stolee <dstolee@microsoft.com>

When the core.sparseCheckoutCone config setting was added in 879321eb0b ("sparse-checkout: add 'cone' mode" 2019-11-21), the variables storing the config values for core.sparseCheckout and core.sparseCheckoutCone were rearranged in cache.h, but in doing so the "extern" keyword was dropped.

While we are tending to drop the "extern" keyword for function declarations, it is still necessary for global variables used across multiple *.c files. The impact of not having the extern keyword may be unpredictable.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 cache.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Show changes to cache.h +2 −2

diff --git a/cache.h b/cache.h
index 1554488d66..cbfaead23a 100644
--- a/cache.h
+++ b/cache.h
@@ -958,8 +958,8 @@ extern int protect_hfs;
 extern int protect_ntfs;
 extern const char *core_fsmonitor;
 
-int core_apply_sparse_checkout;
-int core_sparse_checkout_cone;
+extern int core_apply_sparse_checkout;
+extern int core_sparse_checkout_cone;
 
 /*
  * Include broken refs in all ref iterations, which will

-- 
gitgitgadget

Junio C Hamano· Jan 2, 2020, 18:17 UTC · re: Derrick Stolee via GitGitGadget · lore

Re: [PATCH 1/1] sparse-checkout: use extern for global variables

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 12 quoted lines

> From: Derrick Stolee <dstolee@microsoft.com>
>
> When the core.sparseCheckoutCone config setting was added in
> 879321eb0b ("sparse-checkout: add 'cone' mode" 2019-11-21), the
> variables storing the config values for core.sparseCheckout and
> core.sparseCheckoutCone were rearranged in cache.h, but in doing
> so the "extern" keyword was dropped.
>
> While we are tending to drop the "extern" keyword for function
> declarations, it is still necessary for global variables used
> across multiple *.c files. The impact of not having the extern
> keyword may be unpredictable.

"May be unpredictable" might be a bit too strong, but I agree that it is better not to rely on the "common extension" these days, and instead make sure variable decls have "extern" in front.

Will queue.  Thanks.

Show 20 quoted lines

> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  cache.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 1554488d66..cbfaead23a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -958,8 +958,8 @@ extern int protect_hfs;
>  extern int protect_ntfs;
>  extern const char *core_fsmonitor;
>  
> -int core_apply_sparse_checkout;
> -int core_sparse_checkout_cone;
> +extern int core_apply_sparse_checkout;
> +extern int core_sparse_checkout_cone;
>  
>  /*
>   * Include broken refs in all ref iterations, which will