[Insight-developers] Dashboard Cleanup: mini0.nlm MacOSX-cross-rosetta : 50 tests failing

[Michel Audette]

Hi Luis,

your value *d* in the bolded part of your suggested code is undefined when
used as an index. It has to be as
d = dim - 1;
l = coord[d];
d--;
for coord to have a valid index, doesn't it?

I propose... (extra check on validity of index d)
#define NRRD_INDEX_GEN(I, coord, size, dim)   \
int d;                                                                 \
d = (dim) - 1;                                                      \
if ( (d) >= 0 )                                                      \
{                                                                       \
  (I) = (coord)[d];                                                \
  d--;                                                                 \
  while( d >= 0 )                                                 \
  {                                                                     \
  (I) = (coord)[d] + (size)[d] * (I);                          \
  d--;                                                                 \
  }                                                                     \
}                                                                       \
and to take out the pragmas.

Seem reasonable?

Michel

On Thu, Jan 28, 2010 at 11:54 AM, Luis Ibanez <luis.ibanez at kitware.com>
wrote:
> Wow...
>
>
> This code is really fragile:
>
>
>>  for (d=(dim)-1, (I)=(coord)[d--];           \
>>       d >= 0;                                \
>>       d--) {                                 \
>>    (I) = (coord)[d] + (size)[d]*(I);
>
>
> It looks like it meant to do:
>
> d = dim - 1;
> l = coord[d];
> d--;
>
> while( d >= 0 )
> {
> I = coord[d] + size[d] * I;
> d--;
> }
>
>
> It probably should be:
>
*> l = coord[ d - 1 ];
> d = dim - 2;*
>
> while( d >= 0 )
> {
> I = coord[d] + size[d] * I;
> d--;
> }
>
>
> I agree with Kevin, in that the compiler
> is most likely flagging a real error here.
>
>
>     Luis
>
>
> --------------------------------------------------------------------
> On Thu, Jan 28, 2010 at 10:34 AM, Kevin H. Hobbs <hobbsk at ohiou.edu> wrote:
>> On 01/28/2010 10:12 AM, Michel Audette wrote:
>>> Hi Kevin,
>>>
>>> I've just committed a new version of reorder, which now includes a
>>> pragma for this warning that was present prior to this week's commits.
>>>
>>> Can you give it a try and report back? Thanks for your kind support.
>>>
>>> Best wishes,
>>>
>>
>> The build is under way on lance.
>>
>> I noticed that the same warning is happening in subset.c near the same
>> macro NRRD_INDEX_GEN.
>>
>> I took a look at this macro and it scares me a little because of the use
>> of the post decrement on d as part of the access to an index as part of
>> a for loop... I think the compiler is really trying to tell us
>> something. I at least am having a hard time telling what dim has to be
>> to to avoid ever trying to access coord[-1]...
>>
>> ******** NRRD_INDEX_GEN
>> **
>> ** Given a coordinate array "coord", as well as the array sizes "size"
>> ** and dimension "dim", calculates the linear index, and stores it in
>> ** "I".
>> */
>> #define NRRD_INDEX_GEN(I, coord, size, dim)   \
>> do {                                          \
>>  int d;                                      \
>>  for (d=(dim)-1, (I)=(coord)[d--];           \
>>       d >= 0;                                \
>>       d--) {                                 \
>>    (I) = (coord)[d] + (size)[d]*(I);         \
>>  }                                           \
>> } while (0)
>>
>>
>



-- 
Michel Audette, Ph.D.
R & D Engineer,
Kitware Inc.,
Chapel Hill, N.C.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100128/1b8da41d/attachment.htm>