pgsql: Initialize unused ExprEvalStep fields.

Started by Andres Freundover 8 years ago3 messagescomitters
Jump to latest
#1Andres Freund
andres@anarazel.de

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(-)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#1)
Re: pgsql: Initialize unused ExprEvalStep fields.

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

#3Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#2)
Re: pgsql: Initialize unused ExprEvalStep fields.

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