[Insight-developers] Nonportable Code

Sat Aug 15 12:31:11 EDT 2009

Hi Gaetan,

The logic of the computation in :

http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Common/itkImageBase.txx?root=Insight&r1=1.56&r2=1.60

we flawed anyways.

If the purpose is to verify whether the request for a
number in pixels along every dimension results in
a combined allocation that is larger that the largest
number of pixels that can be allocated, then such
evaluation can't simply be performed at the output
of the for loop.

Instead it should have been tested at every increment
of the "num" value.

Otherwise, by the time the num value gets larger that
the container type, it will wrap around and it will be
presented as a negative number.

The check should have been of the form:

Instead of:
        num *= bufferSize[i];

do:

      if ( NumericTraits<SizeValueType>::max() / buffersize[i] > num )
         {
         itkExceptionMacro();
         }
     num *= bufferSize[i];

all operations in an integer type are equivalent
to the same operation module Integer::Max.

Checking after the fact will not reveal an overflow.
(unless we had a type that is largest that size_t,
which is not the case).

     Luis

-------------------------------------------------------------
2009/8/15 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>

>
> Le 15 août 09 à 05:30, Bill Lorensen a écrit :
>
>  vxl_int_64 must be void on some platforms.
>>
>> http://www.cdash.org/CDash/viewBuildError.php?buildid=402232
>>
>> I repeat my concerns about such fundamental changes so close to a release.
>>
>
> I have commented the changes and filed bug reports to keep a track.
>
>  http://www.itk.org/Bug/view.php?id=9427
>  http://www.itk.org/Bug/view.php?id=9426
>
> We are back to the previous situation.
>
>
> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Common/itkImageBase.txx?root=Insight&r1=1.56&r2=1.60
>
> Gaëtan
>
>
>
>> Bill
>>
>> On Fri, Aug 14, 2009 at 1:25 PM, Tom Vercauteren<tom.vercauteren at m4x.org>
>> wrote:
>>
>>> Hi Sean,
>>>
>>> Yes c++0x will have cstdint. It is already in boost.
>>>
>>> In the meantime we might want to rely on pstdint.h (a portable stdint):
>>> http://www.azillionmonkeys.com/qed/pstdint.h
>>>
>>> See wikipedia for more information
>>> http://en.wikipedia.org/wiki/Stdint.h
>>>
>>> But it looks like a large change to do right before the release.
>>>
>>> Tom
>>>
>>> On Fri, Aug 14, 2009 at 19:21, Sean McBride<sean at rogue-research.com>
>>> wrote:
>>>
>>>> On 8/14/09 6:56 PM, Gaëtan Lehmann said:
>>>>
>>>>  I've used that option, but I'm quite sure the types defined there are
>>>>> wrong.
>>>>> For example,
>>>>>
>>>>>  #ifdef _WIN32
>>>>>    typedef long      ITK_INT64;
>>>>>  #endif
>>>>>
>>>>> and
>>>>>
>>>>>  #ifdef _WIN32
>>>>>    typedef unsigned long ITK_UINT64;
>>>>>  #endif
>>>>>
>>>>> don't like right to me.
>>>>>
>>>>
>>>> Agreed.
>>>>
>>>>  Tom's way - vxl_int_64 - seems to be a better option.
>>>>>
>>>>> Should we fix the content of itkIntType.h by using vxl_int_64 types?
>>>>> And do we even need itkIntType.h?
>>>>>
>>>>
>>>> This whole issue of fixed size types was solved and standardised with
>>>> C99 (ISO/IEC 9899:1999), see:
>>>> <http://en.wikipedia.org/wiki/Stdint.h>
>>>>
>>>> I know ITK is C++, not C.  Anyone know if the next C++ standard C++-0x
>>>> will incorporate those types from C99?
>>>>
>>>> If so, seems to me we should use int32_t instead of creating an
>>>> INT_INT32.  (I know some compilers do not have stdint.h, but we can
>>>> create compatible typedefs in those cases.)
>>>>
>>>> --
>>>> ____________________________________________________________
>>>> Sean McBride, B. Eng                 sean at rogue-research.com
>>>> Rogue Research                        www.rogue-research.com
>>>> Mac Software Developer              Montréal, Québec, Canada
>>>>
>>>>
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>>
>>>> Visit other Kitware open-source projects at
>>>> http://www.kitware.com/opensource/opensource.html
>>>>
>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>
>>>>  _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>
>>>  _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Please keep messages on-topic and check the ITK FAQ at:
>> http://www.itk.org/Wiki/ITK_FAQ
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.itk.org/mailman/listinfo/insight-developers
>>
>
> --
> Gaëtan Lehmann
> Biologie du Développement et de la Reproduction
> INRA de Jouy-en-Josas (France)
> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
> http://voxel.jouy.inra.fr  http://www.itk.org
> http://www.mandriva.org  http://www.bepo.fr
>
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090815/eb09c3aa/attachment.htm>