On Wed, Feb 12, 2014 at 3:22 PM, Erik Faye-Lund <kusmabite@xxxxxxxxx> wrote: > On Wed, Feb 12, 2014 at 3:09 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: >> On Wed, Feb 12, 2014 at 8:23 PM, David Kastrup <dak@xxxxxxx> wrote: >>> Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes: >>> >>>> Am 2/12/2014 13:55, schrieb David Kastrup: >>>>> Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes: >>>>> >>>>>> Running test suite of 'next' on Windows fails in t5310-pack-bitmaps with >>>>>> the following symptoms. I haven't followed the topic. Have there been >>>>>> patches floating that addressed the problem in one way or another? >>>>>> >>>>>> (gdb) run >>>>>> Starting program: D:\Src\mingw-git\t\trash >>>>>> directory.t5310-pack-bitmaps/..\..\git.exe rev-list --test-bitmap >>>>>> HEAD >>>>>> [New thread 3528.0x8d4] >>>>>> Bitmap v1 test (20 entries loaded) >>>>>> Found bitmap for 537ea4d3eb79c95f602873b1167c480006d2ac2d. 64 bits >>>>>> / 15873b36 checksum >>>>> >>>>> Does reverting a201c20b41a2f0725977bcb89a2a66135d776ba2 help? >>>> >>>> YES! t5310 passes after reverting this commit. >>> >>> Oh. I just looked through the backtrace until finding a routine >>> reasonably related with the regtest and checked for the last commit >>> changing it, then posted my question. >>> >>> Then I looked through the diff of the patch and considered it >>> unconspicuous. So I commenced reading through earlier commits. >>> >>> I actually don't have a good idea what might be wrong here. The code is >>> somewhat distasteful as it basically uses eword_t and uint64_t >>> interchangeably, but then this does match its current definition. >> >> Perhaps __BYTE_ORDER or __BIG_ENDIAN is misdefined and the ntohll() is skipped? > > That is indeed the case. Looking a bit at it, the whole byte-order detection mess seems insane to me. MinGW falls inside the "defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))"-block, but does not define __BYTE_ORDER. It seems the author of a201c20b41a2f0725977bcb89a2a66135d776ba2 assumes __BYTE_ORDER was guaranteed to always be defined, probably fooled by the following check: #if !defined(__BYTE_ORDER) # error "Cannot determine endianness" #endif However, that check is only performed if we're NOT building with GCC on x86/x64 nor MSVC (which I don't think defined __BYTE_ORDER either) The following would make __BYTE_ORDER a guarantee, and show that MinGW does not define it: ---8<--- diff --git a/compat/bswap.h b/compat/bswap.h index 120c6c1..c73bf0e 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -109,10 +109,6 @@ static inline uint64_t git_bswap64(uint64_t x) # endif #endif -#if !defined(__BYTE_ORDER) -# error "Cannot determine endianness" -#endif - #if __BYTE_ORDER == __BIG_ENDIAN # define ntohll(n) (n) # define htonll(n) (n) @@ -123,6 +119,10 @@ static inline uint64_t git_bswap64(uint64_t x) #endif +#if !defined(__BYTE_ORDER) +# error "Cannot determine endianness" +#endif + /* * Performance might be improved if the CPU architecture is OK with * unaligned 32-bit loads and a fast ntohl() is available. ---8<--- But another level of insanity (IMO) is defining double-underscore macros. These symbols are reserved for the implementation. Sure, knowing we're on a given implementation and defining something *else* dependent on them is fine. But defining them is just yuckiness, and not very standard-kosher. IMO, we should rather do the definition the other way around: #if !defined(BYTE_ORDER) # if defined(__BYTE_ORDER) && defined(__LITTLE_ENDIAN) && defined(__BIG_ENDIAN) # define BYTE_ORDER __BYTE_ORDER # define LITTLE_ENDIAN __LITTLE_ENDIAN # define BIG_ENDIAN __BIG_ENDIAN # endif #endif ...and only referred to BYTE_ORDER, LITTLE_ENDIAN and BIG_ENDIAN. That way we'd follow closer to where opengroup are heading: http://www.opengroup.org/austin/docs/austin_514.txt -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html