Potential for bugs while using COPY_POINTER_FIELD to copy NULL pointer
Hi,
COPY_POINTER_FIELD is defined as -
61 #define COPY_POINTER_FIELD(fldname, sz) \
62 do { \
63 Size _size = (sz); \
64 newnode->fldname = palloc(_size); \
65 memcpy(newnode->fldname, from->fldname, _size); \
66 } while (0)
Since we allocate _size memory irrespective of whether from->fldname is
NULL, every NULL pointer can get copied as non-NULL pointer because the way
*alloc routines handle 0 sizes.
-- from man malloc
If size is 0, then malloc() returns either NULL, or a unique pointer
value that can later be successfully passed to free()
--
After such a copy tests like if (pointer) will start failing. There are few
callers of COPY_POINTER_FIELD which do not call the macro if the size can
be 0. But there are some who do not do so. This looks fishy, in case we
have if (pointer) kinds of cases.
Shouldn't COPY_POINTER_FIELD return NULL, if the pointer to be copied is
NULL?
--
Best Wishes,
Ashutosh Bapat
EntepriseDB Corporation
The Enterprise Postgres Company
Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
After such a copy tests like if (pointer) will start failing. There are few
callers of COPY_POINTER_FIELD which do not call the macro if the size can
be 0. But there are some who do not do so. This looks fishy, in case we
have if (pointer) kinds of cases.
I don't think we do. That macro is only used to copy fixed-length
support arrays like sort column numbers. There would be no reason to
test such a field for null-ness; its size is always determined by other
properties of the node.
It does look like all the actual uses of the macro are protected by
if-tests if the number of columns could be zero (except for MergeJoin
which didn't use to support zero columns but now does; should go fix
that). But AFAICS that is purely to save a couple of cycles in the copy
operation, not because it would matter later.
regards, tom lane