[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Plans for ITK version 4



On Mon, 2012-04-23 at 00:50 -0500, Steve M. Robbins wrote:
> On Mon, Mar 19, 2012 at 04:46:42PM -0400, Paul Novotny wrote:
> > On Sun, 2012-03-04 at 13:09 -0600, Steve M. Robbins wrote:
> > > Other architectures show a regression in that now the libraries don't
> > > even build whereas before they did build.  Still others claim a
> > > dependency installability problem for minc or gdcm.
> > > 
> > > [1] https://buildd.debian.org/status/package.php?p=insighttoolkit4&suite=experimental
> > 
> > 
> > Here is a one line patch that should help ITK get past the current build
> > errors on big-endian systems (I think). How do you test on these
> > architectures? VMs? This probably isn't necessary in the long run, since
> > I think you are going to use the system TIFF.
> > 
> > -Paul
> 
> > diff --git a/debian/patches/big_endian.patch b/debian/patches/big_endian.patch
> > new file mode 100644
> > index 0000000..2b2d90c
> > --- /dev/null
> > +++ b/debian/patches/big_endian.patch
> > @@ -0,0 +1,11 @@
> > +--- a/Modules/ThirdParty/TIFF/src/itktiff/tif_predict.c
> > ++++ b/Modules/ThirdParty/TIFF/src/itktiff/tif_predict.c
> > +@@ -560,7 +560,7 @@
> > +   for (count = 0; count < wc; count++) {
> > +     uint32 byte;
> > +     for (byte = 0; byte < bps; byte++) {
> > +-      #if WORDS_BIGENDIAN
> > ++      #ifdef WORDS_BIGENDIAN
> 
> I don't understand this change.  On the big endian systems,
> surely something is #defining WORDS_BIGENDIAN.  What is
> it defined as and where is the define?  I'd be surprised
> if the #define was no to "1", in which case #if and #ifdef
> are both true, so what does this change accomplish?

I don't know the intricacies of #if, #ifdef etc. But if WORDS_BIGENDIAN
is defined like this 

#define WORDS_BIGENDIAN

When WORDS_BIGENDIAN is not defined, an #if/#else using it compiles, and
just runs the #else. When it is defined, #if gives you the following
error:

error: #if with no expression

Which is what we are seeing in the build logs for big endian systems
like powerpc, and not seeing on little endian systems. It is pretty
clear the intent of the code is to check if WORDS_BIGENDIAN is defined,
not what it's value is (which it has no value, it is just defined). So
an #ifdef should be used here, as it is done this way earlier in the
same file 

Modules/ThirdParty/TIFF/src/itktiff/tif_predict.c line 394

Actually, I just looked a little bit deeper, and WORDS_BIGENDIAN is
defined in Modules/ThirdParty/TIFF/src/itktiff/tif_config.h.in:187-193.
It looks like it is defined as 1 if AC_APPLE_UNIVERSAL_BUILD and
__BIG_ENDIAN is defined, otherwise it grabs cmakes value for
WORDS_BIGENDIAN (using #cmakedefine). 

CMake uses TestBigEndian.cmake to define WORDS_BIGENDIAN, sets it to 1
or 0. BUT, #cmakedefine doesn't propagate this value. From the cmake man
page:

Any occurrences of #cmakedefine VAR will be replaced with either #define
VAR or /* #undef VAR */ depending on the setting of VAR in CMake. Any
occurrences of #cmakedefine01 VAR will be replaced with either #define
VAR 1 or #define VAR 0 depending on whether VAR evaluates to TRUE or
FALSE in CMake.

SO, in the end, I learned more than I wanted. And the solution is
probably to include my patch, changing #if to #ifdef. But also change
#cmakedefine to #cmakedefine01 in tif_config.h.in.

-Paul




Reply to: