Thread: Potential for bugs while using COPY_POINTER_FIELD to copy NULL pointer

Hi,<br />COPY_POINTER_FIELD is defined as - <br />  61 #define COPY_POINTER_FIELD(fldname, sz) \<br />  62     do {
\<br/>  63         Size    _size = (sz); \<br />  64         newnode->fldname = palloc(_size); \<br />  65        
memcpy(newnode->fldname,from->fldname, _size); \<br />   66     } while (0)<br /><br />Since we allocate _size
memoryirrespective of whether from->fldname is NULL, every NULL pointer can get copied as non-NULL pointer because
theway *alloc routines handle 0 sizes.<br />-- from man malloc<br /> If size  is  0,  then  malloc()  returns either
NULL,or a unique pointer value that can later be successfully passed to free()<br />--<br /><br />After such a copy
testslike if (pointer) will start failing. There are few callers of COPY_POINTER_FIELD which do not call the macro if
thesize can be 0. But there are some who do not do so. This looks fishy, in case we have if (pointer) kinds of
cases.<br/><br />Shouldn't COPY_POINTER_FIELD return NULL, if the pointer to be copied is NULL?<br />-- <br />Best
Wishes,<br/>Ashutosh Bapat<br />EntepriseDB Corporation<br />The Enterprise Postgres Company<br /><br /> 

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