On Wed, Jul 18, 2012, at 11:42 PM, Bron Gondwana wrote:
> On Wed, Jul 18, 2012, at 05:03 PM, Jenkins wrote:
> > Running unit tests
> > ./cunit-to-junit.pl: ran 278 tests, 2 failed
> > make[3]: *** [check-local] Error 1
> > make[3]: Leaving directory
> > `<http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'>
> > make[2]: *** [check-am] Error 2
> > make[2]: Leaving directory
> > `<http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'>
> > make[1]: *** [check-recursive] Error 1
> > make[1]: Leaving directory
> > `<http://ci.cyrusimap.org/job/cyrus-imapd-master/ws/imapd'>
> > make: *** [check] Error 2
> > + fatal 'Can'\''t make check'
> > + echo 'imapd/tools/jenkins-build.sh: Can'\''t make check'
> > imapd/tools/jenkins-build.sh: Can't make check
>
> Of course it bloody runs clean on my machine. Short of logging in and...
>
> ./cunit/byteorder64.testc:12: CU_ASSERT_EQUAL(ntohll(*((uint64_t
> *)(b64)))=72057594037927936,1=1)
>
> Oh shit.
>
> So what's different from my machine? The architecture certainly isn't.
> This means that cmu/master is super-dangerous right now for anyone not
> building on the same platform as me.
>
> Greg - any ideas?? It should be using be64toh from endian.h on Linux
> according to the code.
Hmm.
static void test_byteorder(void)
{
const char b64[8] = { 0, 0, 0, 0, 0, 0, 0, 1 };
const char b32[4] = { 0, 0, 0, 1 };
char i64[8];
char i32[4];
Here's your first problem, the alignment of all these variables is
uncontrolled. The code is only working by accident because the C
runtime environment gives you an aligned stack, and also because x86
doesn't care so much about alignment.
/* test 64 bit */
CU_ASSERT_EQUAL(ntohll(*((uint64_t *)(b64))), 1);
*((uint64_t *)i64) = htonll(1);
CU_ASSERT_EQUAL(memcmp((char *)i64, b64, 8), 0);
/* test 32 bit */
CU_ASSERT_EQUAL(ntohl(*((uint32_t *)(b32))), 1);
*((uint32_t *)i32) = htonl(1);
CU_ASSERT_EQUAL(memcmp((char *)i32, b32, 4), 0);
}
In the header file lib/byteorder64.h
/* http://stackoverflow.com/a/4410728/94253 */
#if defined(__linux__)
# include <endian.h>
#elif defined(__FreeBSD__) || defined(__NetBSD__)
# include <sys/endian.h>
#elif defined(__OpenBSD__)
# include <sys/types.h>
# define be16toh(x) betoh16(x)
# define be32toh(x) betoh32(x)
# define be64toh(x) betoh64(x)
#elif defined(WORDS_BIGENDIAN)
#define CYRUS_BYTESWAP
#endif
That could do with some improvement.
Solaris also has an optimised ntohll() in libc which uses the 64b bswap
instruction, although it's not inlined.
http://cvs.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/amd64/gen/byteorder.s
and gcc has a builtin which uses the instruction on platforms which have
it
http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-g_t_005f_005fbuiltin_005fbswap64-3333
which presumably will work on some platforms where Linux's <endian.h>
(which re-implements the same thing differently) is not available.
Disassembling cunit/byteorder64.o, I don't see any bswap instructions in
there, but nm shows undefined symbols for libc's htonl and ntohl.
Writing a function that calls ntohll() and nothing else, it seems that
ntohll() expands to a no-op. Running gcc -E confirms this.
Aha..
#ifdef be64toh
#define htonll(x) htobe64(x)
#define ntohll(x) be64toh(x)
#elif defined (CYRUS_BYTESWAP)
/* little-endian 64-bit host/network byte-order swap functions */
extern unsigned long long _htonll(unsigned long long);
extern unsigned long long _ntohll(unsigned long long);
#define htonll(x) _htonll(x)
#define ntohll(x) _ntohll(x)
#else
#define htonll(x) (x)
#define ntohll(x) (x)
#endif
The <endian.h> on my desktop defines a be64toh(). The same header on
ci.cyrusimap.org doesn't.
--
Greg.
