[Insight-developers] nrrd assumptions and signed overflow

[Williams, Norman K]

Be forewarned this is a tactless and rude critique of the NRRD library
which, other than the faults described below, is a very useful and robust
library.  If you're on the TEEM team, stop reading now.

nrrdSanity (according to comments) is supposed to "makes sure that all the
basic assumptions of nrrd hold foR the architecture/etc which we're
currently running on."

Aside from awkwardly ending a sentence with a preposition, doing this test
at runtime is, not to put too fine a point on it, idiotic. Performing this
test once during configuration should be sufficient; doing it at runtime

every time you try to open a NRRD image is ridiculous.



The NRRD tests make it so that floating point exceptions need to be
suppressed within the NRRD ImageIO driver.  Writing code ON PURPOSE that
will only work when floating point exceptions are ignored is stupid.


Furthermore code of the form "assign N to X, and succeed if X != N" is
blindingly fugly.

I presume CLang is testing for known overflows at compile time.  Does this
get flagged as a compile error?

Could we just #ifdef out this merde in the ITK tree, since ITK carries
around its own NRRDIO?

On 11/25/11 5:17 PM, "Sean McBride" <sean at rogue-research.com> wrote:

>Hi all,
>
>Many of the "nrrd" tests are failing on my clang dashboard, and I took a
>look at why... There is this function:
>
>****** nrrdSanity
>**
>** makes sure that all the basic assumptions of nrrd hold for
>** the architecture/etc which we're currently running on.
>**
>** returns 1 if all is okay, 0 if there is a problem
>
>which does this:
>
>  tmpLLI = NRRD_LLONG_MAX;
>  if (tmpLLI != NRRD_LLONG_MAX) {
>    return 0;
>  }
>
>  tmpLLI += 1; // problem here
>
>  if (NRRD_LLONG_MIN != tmpLLI) {
>    return 0;
>  }
>  tmpULLI = NRRD_ULLONG_MAX;
>
>"tmpLLI" is a signed long long.  They are adding 1 to the largest
>possible signed long long, and signed overflow is undefined in C/C++.
>The compiler can do whatever it wants in the case of undefined behaviour,
>and with the flags I'm using it deliberately generates an illegal
>instruction.
>
>I stopped investigating here, since from the comments of this method, it
>seems that nrrd requires/expects that the compiler wraps signed values.
>
>Anyone know this nrrd stuff well?
>
>Cheers,
>
>--
>____________________________________________________________
>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
>
>Kitware offers ITK Training Courses, for more information visit:
>http://kitware.com/products/protraining.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



________________________________
Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
________________________________