search

Amrnasr / lz4

Automatically exported from code.google.com/p/lz4
0 stars 0 forks source link

Portability patches #37

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago 
As part of testing out LZ4 I have compiled and run it on a
collection of machines on our build cluster including some with
very old compilers.

I needed to make a number of changes in order for lz4.c to
compile and work everywhere.  I did not test lz4hc.c, but it
probably needs a matching set of changes.

The build cluster includes:
   x86 linux on Debian: 32 & 64bit versions of all releases since 3.0
   x86 freebsd: all 32 & 64bit versions since 6.0
   netbsd 1.6.1 on 32bit x86
   netbsd 4.0 on 32/64bit x86
   OpenBSD 4.2 on x86
   linux on ia64 & PowerPC
   Solaris on x86 and Sparc
   SCO OpenServer R5 on x86
   MacOS 10.3 PowerPC
   MacOS 10.4 x86
   AIX 5 on PowerPC
   HP-UX on PARISC and IA-64

   and probably some other oddballs that I forgot about

Here were the changes I needed to make

64-bit detection:

  The define for 64-bit code was missing __ia64__

BIG_ENDIAN:

  For big endian I added __hpux__ && __sparc__ 
  I didn't test this part, but it would probably be a good idea to
  also add a test of:
    && !(defined(__BYTE_ORDER__) &&
     (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__))

  The powerpc can actually be either, but in most common cases
  it is big endian so that part of the test seems to work.

Packed structures:

  The '#pragma pack(1)' directive for the U32_S structures doesn't
  work in older GCCs or with the ia64 architecture.
  In my searching I found the __packed__ attribute was more portable,
  so I switched to using that instead.  Not sure if that will
  work with MSVC or not as that is not one of my targets.

stdint.h

  The stdint.h is too new to be included on all of these machines so
  used typedefs directly. (why #defines before?)
  In theory these defines can be wrong, but in practice it works on
  every architecture in common use.

Original issue reported on code.google.com by wsc...@gmail.com on 5 Oct 2012 at 2:26

Attachments:

GoogleCodeExporter commented 8 years ago 
Very interesting insight.
I'll look into it.

Original comment by yann.col...@gmail.com on 5 Oct 2012 at 2:29

GoogleCodeExporter commented 8 years ago 
For 64-bit it should be enough to test for __LP64__.
The also had problems with endian on sparc (When using the Solaris Studio 
compiler). I just moved the include files up to before the endian test.

The problem did not show under compilation though. Just that it did not work.

Original comment by thwill...@gmail.com on 7 Oct 2012 at 3:07

GoogleCodeExporter commented 8 years ago 
I would like to open again this issue and try to include it into next release.

64-bit detection: __ia64__ will be added to the list. Thanks.

BIG_ENDIAN: the big endian macro detection has been greatly improved since this 
issue was raised. hpux & sparc are now part of the detection formula.
I suspect it would work as it is today, although nothing would replace a real 
test.

Packed structures: the problem with __packed__ attribute is that it is not 
supported outside of GCC. #pragma pack(1) is, overall, better supported, 
although not perfect, as proved by your tests. Maybe a more complex combination 
of both would be necessary, although i fear it could raise code complexity.

stdint.h: Understood that older compilers may not support it.
These direct definitions were the one I originally used. I then received a few 
comments stating that it was "not proper way" to declare such types, and 
switched to it. So now, it seems there are conflicting advises :)
Not sure to understand the comment regarding #defines, but i guess you mean 
typedef should be preferred.

Original comment by yann.col...@gmail.com on 29 Apr 2013 at 8:01

GoogleCodeExporter commented 8 years ago 
Regarding basic types :
decided to move to the following version :

#if __STDC_VERSION__ >= 199901L    // C99
# include <stdint.h>
  typedef uint8_t  BYTE;
  typedef uint16_t U16;
  typedef uint32_t U32;
  typedef  int32_t S32;
  typedef uint64_t U64;
#else
  typedef unsigned char       BYTE;
  typedef unsigned short      U16;
  typedef unsigned int        U32;
  typedef   signed int        S32;
  typedef unsigned long long  U64;
#endif

It seems proper : stdint.h is guaranteed by C99. For all other compilers, the 
direct typedef you propose looks the best bet.

Original comment by yann.col...@gmail.com on 29 Apr 2013 at 8:34

GoogleCodeExporter commented 8 years ago 
I'll handle it.

Original comment by yann.col...@gmail.com on 29 Apr 2013 at 8:35

GoogleCodeExporter commented 8 years ago 
The attached file is an LZ4 release candidate,
which is expected to solve all issues mentioned in this thread,
except pragma-pack.

Original comment by yann.col...@gmail.com on 10 May 2013 at 12:37

Attachments:

GoogleCodeExporter commented 8 years ago 
The following release candidate is supposed to solve all issues mentioned in 
this thread, including the #pragma pack directive one.

Original comment by yann.col...@gmail.com on 16 May 2013 at 9:10

Attachments:

GoogleCodeExporter commented 8 years ago 
Corrected into r95

Original comment by yann.col...@gmail.com on 17 May 2013 at 6:45