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

From: Jonathan Tan <jonathantanmy@google.com>
Date: Dec 18, 2017, 23:43 UTC
Message-ID: <20171218154310.aa62ea2c9f094b663393556e@google.com>
In-Reply-To: <20171218214947.210042-1-sbeller@google.com>

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.