threads / patch / 47460

patcht/helper/test-lazy-name-hash: fix compilation

Subject: [FYI PATCH] t/helper/test-lazy-name-hash: fix compilation

## tl;dr

7 messages between Dec 18, 2017 and Dec 22, 2017. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Stefan Beller· Dec 18, 2017, 21:49 UTC · lore

I was compiling origin/master today with stricter compiler flags today and was greeted by

t/helper/test-lazy-init-name-hash.c: In function ‘cmd_main’:
t/helper/test-lazy-init-name-hash.c:172:5: error: ‘nr_threads_used’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     printf("avg [size %8d] [single %f] %c [multi %f %d]\n",
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         nr,
         ~~~
         (double)avg_single/1000000000,
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         (avg_single < avg_multi ? '<' : '>'),
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         (double)avg_multi/1000000000,
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         nr_threads_used);
         ~~~~~~~~~~~~~~~~
t/helper/test-lazy-init-name-hash.c:115:6: note: ‘nr_threads_used’ was declared here
  int nr_threads_used;
      ^~~~~~~~~~~~~~~

I do not see how we can arrive at that line without having `nr_threads_used` initialized, as we'd have `count > 1` (which asserts that we ran the loop above at least once, such that it *should* be initialized).

I do not have time to dive into further analysis.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/helper/test-lazy-init-name-hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/helper/test-lazy-init-name-hash.c +1 −1
diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c
index 6368a89345..297fb01d61 100644
--- a/t/helper/test-lazy-init-name-hash.c
+++ b/t/helper/test-lazy-init-name-hash.c
@@ -112,7 +112,7 @@ static void analyze_run(void)
 {
 	uint64_t t1s, t1m, t2s, t2m;
 	int cache_nr_limit;
-	int nr_threads_used;
+	int nr_threads_used = 0;
 	int i;
 	int nr;
 
-- 
2.15.1.620.gb9897f4670-goog
Jonathan Tan· Dec 18, 2017, 23:43 UTC · re: Stefan Beller · lore

Re: [FYI PATCH] t/helper/test-lazy-name-hash: fix compilation

On Mon, 18 Dec 2017 13:49:47 -0800 Stefan Beller <sbeller@google.com> wrote:

Show 24 quoted lines
> I was compiling origin/master today with stricter compiler flags today
> and was greeted by
> 
> t/helper/test-lazy-init-name-hash.c: In function ‘cmd_main’:
> t/helper/test-lazy-init-name-hash.c:172:5: error: ‘nr_threads_used’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      printf("avg [size %8d] [single %f] %c [multi %f %d]\n",
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          nr,
>          ~~~
>          (double)avg_single/1000000000,
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          (avg_single < avg_multi ? '<' : '>'),
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          (double)avg_multi/1000000000,
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          nr_threads_used);
>          ~~~~~~~~~~~~~~~~
> t/helper/test-lazy-init-name-hash.c:115:6: note: ‘nr_threads_used’ was declared here
>   int nr_threads_used;
>       ^~~~~~~~~~~~~~~
> 
> I do not see how we can arrive at that line without having `nr_threads_used`
> initialized, as we'd have `count > 1`  (which asserts that we ran the
> loop above at least once, such that it *should* be initialized).

Your analysis makes sense. (The compiler probably couldn't detect it because "count" is a static variable, not a local variable.)

Show 10 quoted lines
> --- a/t/helper/test-lazy-init-name-hash.c
> +++ b/t/helper/test-lazy-init-name-hash.c
> @@ -112,7 +112,7 @@ static void analyze_run(void)
>  {
>  	uint64_t t1s, t1m, t2s, t2m;
>  	int cache_nr_limit;
> -	int nr_threads_used;
> +	int nr_threads_used = 0;
>  	int i;
>  	int nr;

I agree that this is probably the best way to fix it. Another way might be to omit printing out the number of threads used in the printf that prints the average statistics.

The best way is probably to not use so many global variables, but that is out of scope of this change.

Jeff Hostetler· Dec 19, 2017, 14:19 UTC · re: Stefan Beller · lore

Re: [FYI PATCH] t/helper/test-lazy-name-hash: fix compilation

On 12/18/2017 4:49 PM, Stefan Beller wrote:
Show 44 quoted lines
> I was compiling origin/master today with stricter compiler flags today
> and was greeted by
> 
> t/helper/test-lazy-init-name-hash.c: In function ‘cmd_main’:
> t/helper/test-lazy-init-name-hash.c:172:5: error: ‘nr_threads_used’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>       printf("avg [size %8d] [single %f] %c [multi %f %d]\n",
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           nr,
>           ~~~
>           (double)avg_single/1000000000,
>           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           (avg_single < avg_multi ? '<' : '>'),
>           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           (double)avg_multi/1000000000,
>           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>           nr_threads_used);
>           ~~~~~~~~~~~~~~~~
> t/helper/test-lazy-init-name-hash.c:115:6: note: ‘nr_threads_used’ was declared here
>    int nr_threads_used;
>        ^~~~~~~~~~~~~~~
> 
> I do not see how we can arrive at that line without having `nr_threads_used`
> initialized, as we'd have `count > 1`  (which asserts that we ran the
> loop above at least once, such that it *should* be initialized).
> 
> I do not have time to dive into further analysis.
> 
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>   t/helper/test-lazy-init-name-hash.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c
> index 6368a89345..297fb01d61 100644
> --- a/t/helper/test-lazy-init-name-hash.c
> +++ b/t/helper/test-lazy-init-name-hash.c
> @@ -112,7 +112,7 @@ static void analyze_run(void)
>   {
>   	uint64_t t1s, t1m, t2s, t2m;
>   	int cache_nr_limit;
> -	int nr_threads_used;
> +	int nr_threads_used = 0;
>   	int i;
>   	int nr;
Agreed. It should not be a problem.  Explicitly initializing it is fine.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Stefan Beller· Dec 20, 2017, 22:24 UTC · re: Jeff Hostetler · lore

[PATCH] t/helper/test-lazy-name-hash: fix compilation

I was compiling origin/master today with the DEVELOPER compiler flags today and was greeted by

t/helper/test-lazy-init-name-hash.c: In function ‘cmd_main’:
t/helper/test-lazy-init-name-hash.c:172:5: error: ‘nr_threads_used’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     printf("avg [size %8d] [single %f] %c [multi %f %d]\n",
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         nr,
         ~~~
         (double)avg_single/1000000000,
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         (avg_single < avg_multi ? '<' : '>'),
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         (double)avg_multi/1000000000,
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         nr_threads_used);
         ~~~~~~~~~~~~~~~~
t/helper/test-lazy-init-name-hash.c:115:6: note: ‘nr_threads_used’ was declared here
  int nr_threads_used;
      ^~~~~~~~~~~~~~~
Fix this issue by assigning 0 to 'nr_threads_used'.
Acked-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---

Slightly reworded the commit message. I'd really like this patch to be included such that I can compile git with the DEVELOPER_CFLAGS flags.

 t/helper/test-lazy-init-name-hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to t/helper/test-lazy-init-name-hash.c +1 −1
diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c
index 6368a89345..297fb01d61 100644
--- a/t/helper/test-lazy-init-name-hash.c
+++ b/t/helper/test-lazy-init-name-hash.c
@@ -112,7 +112,7 @@ static void analyze_run(void)
 {
 	uint64_t t1s, t1m, t2s, t2m;
 	int cache_nr_limit;
-	int nr_threads_used;
+	int nr_threads_used = 0;
 	int i;
 	int nr;
 
-- 
2.15.1.620.gb9897f4670-goog
Junio C Hamano· Dec 22, 2017, 18:15 UTC · re: Stefan Beller · lore

Re: [PATCH] t/helper/test-lazy-name-hash: fix compilation

Stefan Beller <sbeller@google.com> writes:
Show 30 quoted lines
> I was compiling origin/master today with the DEVELOPER compiler flags
> today and was greeted by
>
> t/helper/test-lazy-init-name-hash.c: In function ‘cmd_main’:
> t/helper/test-lazy-init-name-hash.c:172:5: error: ‘nr_threads_used’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>      printf("avg [size %8d] [single %f] %c [multi %f %d]\n",
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          nr,
>          ~~~
>          (double)avg_single/1000000000,
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          (avg_single < avg_multi ? '<' : '>'),
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          (double)avg_multi/1000000000,
>          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>          nr_threads_used);
>          ~~~~~~~~~~~~~~~~
> t/helper/test-lazy-init-name-hash.c:115:6: note: ‘nr_threads_used’ was declared here
>   int nr_threads_used;
>       ^~~~~~~~~~~~~~~
>
> Fix this issue by assigning 0 to 'nr_threads_used'.
>
> Acked-by: Jonathan Tan <jonathantanmy@google.com>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Slightly reworded the commit message. I'd really like this patch to be included
> such that I can compile git with the DEVELOPER_CFLAGS flags.

Heh; I do not think there particularly is much difference between stricter flags and DEVELOPER flags, but I would rather not lose the removal of duplicated 'today' I did while I queued the previous one ;-)

Show 16 quoted lines
>
>  t/helper/test-lazy-init-name-hash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/helper/test-lazy-init-name-hash.c b/t/helper/test-lazy-init-name-hash.c
> index 6368a89345..297fb01d61 100644
> --- a/t/helper/test-lazy-init-name-hash.c
> +++ b/t/helper/test-lazy-init-name-hash.c
> @@ -112,7 +112,7 @@ static void analyze_run(void)
>  {
>  	uint64_t t1s, t1m, t2s, t2m;
>  	int cache_nr_limit;
> -	int nr_threads_used;
> +	int nr_threads_used = 0;
>  	int i;
>  	int nr;
Stefan Beller· Dec 22, 2017, 18:17 UTC · re: Junio C Hamano · lore

Re: [PATCH] t/helper/test-lazy-name-hash: fix compilation

> Heh; I do not think there particularly is much difference between
> stricter flags and DEVELOPER flags, but I would rather not lose the
> removal of duplicated 'today' I did while I queued the previous one
> ;-)
I did not realize it was queued anywhere, please ignore then.
Junio C Hamano· Dec 22, 2017, 20:02 UTC · re: Stefan Beller · lore

Re: [PATCH] t/helper/test-lazy-name-hash: fix compilation

Stefan Beller <sbeller@google.com> writes:
Show 7 quoted lines
>> Heh; I do not think there particularly is much difference between
>> stricter flags and DEVELOPER flags, but I would rather not lose the
>> removal of duplicated 'today' I did while I queued the previous one
>> ;-)
>
>
> I did not realize it was queued anywhere, please ignore then.

I just did s/stricter/DEVELOPER/ on the one that have been queued. Thanks.

← back to recent threads