On 2015-07-07 16:17:47 +0900, Michael Paquier wrote:
> 2) Potential pointer dereference in plperl.c, fixed by 0002 (sent
> previously here =>
> CAB7nPqRBCWAXTLw0yBR=BK94cRYXU8TWVxGyYoxautw08OKeXw@mail.gmail.com).
> This is related to a change done by transforms. In short,
> plperl_call_perl_func@plperl.c will have a pointer dereference if
> desc->arg_arraytype[i] is InvalidOid. And AFAIK,
> fcinfo->flinfo->fn_oid can be InvalidOid in this code path.
Aren't we in trouble if fn_oid isn't valid at that point? Why would it
be ok for it to be InvalidOid? The function oid is used in lots of
fundamental places, like actually compiling the plperl functions
(compile_plperl_function).
Which path could lead to it validly being InvalidOid?
> 3) visibilitymap_truncate and FreeSpaceMapTruncateRel are doing a
> NULL-pointer check on rel->rd_smgr but it has been dereferenced in all
> the code paths leading to those checks. See 0003. For code readability
> mainly.
FWIW, there's actually some reasoning/paranoia behind those
checks. smgrtruncate() sends out immediate smgr sinval messages, which
will invalidate rd_smgr. Now, I think in both cases there's currently
no way we'll run code between the smgrtruncate() and the if
(rel->rd_smgr) that does a CommandCounterIncrement() causing them to be
replayed locally, but there's some merit in future proofing.
> 4) Return result of timestamp2tm is not checked 2 times in
> GetCurrentDateTime and GetCurrentTimeUsec, while all the other 40
> calls of this function do sanity checks. Returning
> ERRCODE_DATETIME_VALUE_OUT_OF_RANGE in case of an error would be good
> for consistency. See 0004. (issue reported previously here
> CAB7nPqRSk=J8eUdd55fL-w+k=8sDTHLVBt-cgG9jWN=VO2ogBQ@mail.gmail.com)
But the other cases accept either arbitrary input or perform timezone
conversions. Not the case here, no?
- Andres