Thread: pgsql: Initialize unused ExprEvalStep fields.

pgsql: Initialize unused ExprEvalStep fields.

From
Andres Freund
Date:
Initialize unused ExprEvalStep fields.

ExecPushExprSlots didn't initialize ExprEvalStep's resvalue/resnull
steps as it didn't use them. That caused wrong valgrind warnings for
an upcoming patch, so zero-intialize.

Also zero-initialize all scratch ExprEvalStep's allocated on the
stack, to avoid issues with similar future omissions of non-critial
data.

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/fc96c6942551dafa6cb2a6000cbc9b20643e5db3

Modified Files
--------------
src/backend/executor/execExpr.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)


Re: pgsql: Initialize unused ExprEvalStep fields.

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> Initialize unused ExprEvalStep fields.
> ExecPushExprSlots didn't initialize ExprEvalStep's resvalue/resnull
> steps as it didn't use them. That caused wrong valgrind warnings for
> an upcoming patch, so zero-intialize.

Check ...

> Also zero-initialize all scratch ExprEvalStep's allocated on the
> stack, to avoid issues with similar future omissions of non-critial
> data.

Dunno that that's a good idea; it will also serve to hide valid warnings.

            regards, tom lane


Re: pgsql: Initialize unused ExprEvalStep fields.

From
Andres Freund
Date:
Hi,

On 2018-01-29 15:16:59 -0500, Tom Lane wrote:
> > Also zero-initialize all scratch ExprEvalStep's allocated on the
> > stack, to avoid issues with similar future omissions of non-critial
> > data.
> 
> Dunno that that's a good idea; it will also serve to hide valid warnings.

Possible, but I think it's more likely that new fields with default to
zero/NULL anyway. Zero initializing around allocations is a pretty
common thing in the PG codebase, so it does feel consistent to me.

Greetings,

Andres Freund