[Insight-developers] NULL-pointer tests on 64-bit platforms

[Zachary Pincus]

I just committed a bug fix to a filter in ITK which broke the 
continuous builds on geeds, viggen and moxel. Yet on my machine 
(darwin/gcc 3.3), everything worked fine. The fix contained the 
following idiom, which was at fault:
if (m_Image != NULL)
   { do something }

Fortunately I recalled an email (below) about similar code not working 
on x86_64. (Though I'm not sure whether geeds, viggen, or moxel are 
64-bit!)

Now, the email quoted below mentions that code like:
if (m_Image != 0)
was not working because 0 isn't required to be interpreted as a NULL 
pointer. Now, I was surprised when my code didn't work, as I thought 
that 'NULL' was always required to be interpreted as a NULL pointer. 
Clearly this is not the case.

Anyhow, I fixed the problem by changing
if (m_Image != NULL)
   { do something }

to
if (m_Image)
   { do something }

but I personally *quite* dislike this latter form because it is much 
more implicit in what it is checking for.

Does anyone have any idea what's going on here? Will moving to a 64-bit 
platform require that no code of the form smartPointer == NULL or 
smartPointer == 0 ever be used? If so, that would be a real shame, and 
could confuse users who are used to such code working.

Zach Pincus

Department of Biochemistry and Program in Biomedical Informatics
Stanford University School of Medicine

> 3.
> ITK has some problems due to templates definitions that lead to 
> attempts to cast the integer 0 to a pointer of a different size on 
> x86_64.
> e.g. itkSmartPointer.h has a template definition:
>
> template <typename R>
> bool operator != ( R r ) const
> { return (m_Pointer != (ObjectType*)(r) ); }
>
> This cast is not really safe, and gives warning messages such as:
>> [snip]
>
> The problem is there are several places constructs like:
>    if ( this->m_Coefficients != 0 )
> are used.  This calls the != operator for the smart pointer template, 
> which is not required to interpret 0 as a NULL pointer in this 
> context.
> The cast in the operator definition then tries to interpret the 
> integer 0 as a pointer, and complains since they are not the same 
> size.
>
> A stricter template operator definition would replace the cast above 
> with this:
>  { return (m_Pointer != static_cast<ObjectType *>(r) ); }
>
> This then leads to errors being declared where the usage is incorrect, 
> e.g.
>
>> [snip]
>
> The usage can be corrected by using constructs like:
>  if (this->m_Coefficients)
>
>
> I have been wondering if it would be easier to modify the operator 
> definition to interpret its argument as a pointer, or to change all 
> comparisons throughout ITK of the form:
> if (pointer != 0)
>  to
> if(pointer)