On Wed, Jun 7, 2023 at 7:31 AM Tristan Partin <tristan@neon.tech> wrote:
>
> This patch is really not necessary from a functional point of view. It
> is only necessary if we want to silence a compiler warning.
>
> Tested on `gcc (GCC) 13.1.1 20230511 (Red Hat 13.1.1-2)`.
...
> From my perspective, this warning definitely seems like a false
> positive, but I don't know the code well-enough to say that for certain.
It was a bit confusing to see a patch for src/bin/pgbench/pgbench.c,
but that file not mentioned in the warning messages you quoted. I take
it that your patch silences a warning in pgbench.c; it would've been
nice to see the actual warning.
- PgBenchValue vargs[MAX_FARGS];
+ PgBenchValue vargs[MAX_FARGS] = { 0 };
If I'm right about what kind of warning this might've caused (use of
possibly uninitialized variable), you're correct that it is benign.
The for loop after declarations initializes all the elements of this
array using evaluateExpr(), and if the initialization fails for some
reason, the loop ends prematurely and returns from the function.
I analyzed a few code-paths that return true from evaluateExpr(), and
I'd like to believe that _all_ code paths that return true also
initialize the array element passed. But because there are so many
branches and function calls beneath evaluateExpr(), I think it's
better to be paranoid and initialize all the array elements to 0.
Also, it is better to initialize/clear an array at the point of
definition, like your patch does. So, +1 to the patch.
Best regards,
Gurjeet
http://Gurje.et