New compiler warning in jsonb_plpython.c

Started by Tom Laneabout 6 years ago1 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

I see that buildfarm member caiman is generating a warning [1]https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=caiman&dt=2020-01-25%2015%3A00%3A52:

jsonb_plpython.c: In function \xe2\x80\x98PLyObject_ToJsonbValue\xe2\x80\x99:
cc1: warning: function may return address of local variable [-Wreturn-local-addr]
jsonb_plpython.c:413:13: note: declared here
413 | JsonbValue buf;
| ^~~

It wasn't doing that a week or two ago when I last trawled the buildfarm
for warnings ... but this is unsurprising considering that the compiler
it's using is hot off the presses:

configure: using compiler=gcc (GCC) 10.0.1 20200121 (Red Hat 10.0.1-0.4)

The warning is from code like this:

{
JsonbValue buf;
JsonbValue *out;
...
if (*jsonb_state)
out = &buf;
else
out = palloc(sizeof(JsonbValue));
...
return (*jsonb_state ?
pushJsonbValue(jsonb_state, is_elem ? WJB_ELEM : WJB_VALUE, out) :
out);
}

so I can't say I blame gcc for being unhappy. This code is safe as long
as *jsonb_state doesn't change in between, and as long as pushJsonbValue
doesn't expect its last argument to point at non-transient storage. But
gcc doesn't want to assume that, and I don't really like the assumption
either.

I am thinking of trying to silence the warning by changing the return
to be like

return (out == &buf ?
pushJsonbValue(jsonb_state, is_elem ? WJB_ELEM : WJB_VALUE, out) :
out);

If that doesn't work, or if anyone thinks it's too ugly, I think we
should just drop the optimization of avoiding a palloc, and make
this function do a palloc always. It seems unlikely that anyone
would notice a performance difference, and the code would surely
be less rickety.

Thoughts?

regards, tom lane

[1]: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=caiman&dt=2020-01-25%2015%3A00%3A52