Re: htonll, ntohll
On 2013-10-30 22.07, Ramsay Jones wrote: > On 30/10/13 20:30, Torsten Bögershausen wrote: >> On 2013-10-30 20.06, Ramsay Jones wrote: >>> On 30/10/13 17:14, Torsten Bögershausen wrote: On 2013-10-30 18.01, Vicent Martí wrote: > On Wed, Oct 30, 2013 at 5:51 PM, Torsten Bögershausen > wrote: >> There is a name clash under cygwin 1.7 (1.5 is OK) >> The following "first aid hot fix" works for me: >> /Torsten > > If Cygwin declares its own bswap_64, wouldn't it be better to use it > instead of overwriting it with our own? Yes, this will be part of a longer patch. I found that some systems have something like this: #define htobe64(x) bswap_64(x) And bswap_64 is a function, so we can not detect it by "asking" #ifdef bswap_64 .. #endif But we can use #ifdef htobe64 ... #endif and this will be part of a bigger patch. And, in general, we should avoid to introduce functions which may have a name clash. Using the git_ prefix for function names is a good practice. So in order to unbrake the compilation error under cygwin 17, the "hotfix" can be used. >>> >>> heh, my patch (given below) took a different approach, but >>> >>> ATB, >>> Ramsay Jones >>> >>> -- >8 -- >>> Subject: [PATCH] compat/bswap.h: Fix redefinition of bswap_64 error on >>> cygwin >>> MIME-Version: 1.0 >>> Content-Type: text/plain; charset=UTF-8 >>> Content-Transfer-Encoding: 8bit >>> >>> Since commit 452e0f20 ("compat: add endianness helpers", 24-10-2013) >>> the cygwin build has failed like so: >>> >>> GIT_VERSION = 1.8.4.1.804.g1f3748b >>> * new build flags >>> CC credential-store.o >>> In file included from git-compat-util.h:305:0, >>> from cache.h:4, >>> from credential-store.c:1: >>> compat/bswap.h:67:24: error: redefinition of 'bswap_64' >>> In file included from /usr/include/endian.h:32:0, >>> from /usr/include/cygwin/types.h:21, >>> from /usr/include/sys/types.h:473, >>> from /usr/include/sys/unistd.h:9, >>> from /usr/include/unistd.h:4, >>> from git-compat-util.h:98, >>> from cache.h:4, >>> from credential-store.c:1: >>> /usr/include/byteswap.h:31:1: note: previous definition of \ >>> ‘bswap_64’ was here >>> Makefile:1985: recipe for target 'credential-store.o' failed >>> make: *** [credential-store.o] Error 1 >>> >>> Note that cygwin has a defintion of 'bswap_64' in the >>> header file (which had already been included by git-compat-util.h). >>> In order to suppress the error, ensure that the header >>> is included, just like the __GNUC__/__GLIBC__ case, rather than >>> attempting to define a fallback implementation. >>> >>> Signed-off-by: Ramsay Jones >>> --- >>> compat/bswap.h | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/compat/bswap.h b/compat/bswap.h >>> index ea1a9ed..b864abd 100644 >>> --- a/compat/bswap.h >>> +++ b/compat/bswap.h >>> @@ -61,7 +61,7 @@ static inline uint32_t git_bswap32(uint32_t x) >>> # define ntohll(n) (n) >>> # define htonll(n) (n) >>> #elif __BYTE_ORDER == __LITTLE_ENDIAN >>> -# if defined(__GNUC__) && defined(__GLIBC__) >>> +# if defined(__GNUC__) && (defined(__GLIBC__) || defined(__CYGWIN__)) >>> # include >>> # else /* GNUC & GLIBC */ >>> static inline uint64_t bswap_64(uint64_t val) >>> >> >> Nice, much better. >> >> And here comes a patch for a big endian machine. >> I tryied to copy-paste a patch in my mail program, >> not sure if this applies. >> >> -- >8 -- >> Subject: [PATCH] compat/bswap.h: htonll and ntohll for big endian >> >> Since commit 452e0f20 ("compat: add endianness helpers", 24-10-2013) >> the build on a Linux/ppc gave a warning like this: >> CC ewah/ewah_io.o >> ewah/ewah_io.c: In function ‘ewah_serialize_to’: >> ewah/ewah_io.c:81: warning: implicit declaration of function ‘htonll’ >> ewah/ewah_io.c: In function ‘ewah_read_mmap’: >> ewah/ewah_io.c:132: warning: implicit declaration of function ‘ntohll’ >> >> Fix it by placing the #endif for "#ifdef bswap32" at the right place. >> >> Signed-off-by: Torsten Bögershausen >> --- >> compat/bswap.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/compat/bswap.h b/compat/bswap.h >> index ea1a9ed..b4ddab0 >> --- a/compat/bswap.h >> +++ b/compat/bswap.h >> @@ -46,6 +46,7 @@ static inline uint32_t git_bswap32(uint32_t x) >> #undef htonl >> #define ntohl(x) bswap32(x) >> #define htonl(x) bswap32(x) >> +#endif >> >> #ifndef __BYTE_ORDER >> # if defined(BYTE_ORDER) && defined(LITTLE_ENDIAN) && >> defined(BIG_ENDIAN) >> @@ -82,4 +83,3 @@ static inline uint64_t bswap_64(uint64_t val) >> # error "Can't define htonll or ntohll!" >> #endif >> >> -#endif >> >> . >> > >
Re: htonll, ntohll
On 31/10/13 13:24, Torsten Bögershausen wrote: > On 2013-10-30 22.07, Ramsay Jones wrote: [ ... ] >> Yep, this was the first thing I did as well! ;-) (*late* last night) >> >> I haven't had time today to look into fixing up the msvc build >> (or a complete re-write), so I look forward to seeing your solution. >> (do you have msvc available? - or do you want me to look at fixing >> it? maybe in a day or two?) >> > Ramsay, > I don't have msvc, so feel free to go ahead, as much as you can. > > I'll send a patch for the test code I have made, and put bswap.h on hold for > a week > (to be able to continue with t5601/connect.c) Unfortunately, I haven't had much time to look into this. I do have a patch (given below) that works on Linux, cygwin, MinGW and msvc. However, the msvc build is still broken (as a result of _other_ commits in this 'jk/pack-bitmap' branch; as well as the use of a VLA in another commit). So, I still have work to do! :( Anyway, I thought I would send what I have, so you can take a look. Note, that I don't have an big-endian machine to test this on, so YMMV. Indeed, the *only* testing I have done is to run the test added by this branch (t5310-pack-bitmaps.sh), which works on Linux, cygwin and MinGW. [Note: I have never particularly liked htons, htonl et.al., so adding these htonll/ntohll functions doesn't thrill me! :-D For example see this post[1], which echo's my sentiments exactly.] HTH ATB, Ramsay Jones [1] http://commandcenter.blogspot.co.uk/2012/04/byte-order-fallacy.html -- >8 -- Subject: [PATCH] compat/bswap.h: Fix build on cygwin, MinGW and msvc --- compat/bswap.h | 97 -- 1 file changed, 68 insertions(+), 29 deletions(-) diff --git a/compat/bswap.h b/compat/bswap.h index ea1a9ed..c18a78e 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -17,7 +17,20 @@ static inline uint32_t default_swab32(uint32_t val) ((val & 0x00ff) << 24)); } +static inline uint64_t default_bswap64(uint64_t val) +{ + return (((val & (uint64_t)0x00ffULL) << 56) | + ((val & (uint64_t)0xff00ULL) << 40) | + ((val & (uint64_t)0x00ffULL) << 24) | + ((val & (uint64_t)0xff00ULL) << 8) | + ((val & (uint64_t)0x00ffULL) >> 8) | + ((val & (uint64_t)0xff00ULL) >> 24) | + ((val & (uint64_t)0x00ffULL) >> 40) | + ((val & (uint64_t)0xff00ULL) >> 56)); +} + #undef bswap32 +#undef bswap64 #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__)) @@ -32,54 +45,80 @@ static inline uint32_t git_bswap32(uint32_t x) return result; } +#define bswap64 git_bswap64 +#if defined(__x86_64__) +static inline uint64_t git_bswap64(uint64_t x) +{ + uint64_t result; + if (__builtin_constant_p(x)) + result = default_bswap64(x); + else + __asm__("bswap %q0" : "=r" (result) : "0" (x)); + return result; +} +#else +static inline uint64_t git_bswap64(uint64_t x) +{ + union { uint64_t i64; uint32_t i32[2]; } tmp, result; + if (__builtin_constant_p(x)) + result.i64 = default_bswap64(x); + else { + tmp.i64 = x; + result.i32[0] = git_bswap32(tmp.i32[1]); + result.i32[1] = git_bswap32(tmp.i32[0]); + } + return result.i64; +} +#endif + #elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64)) #include #define bswap32(x) _byteswap_ulong(x) +#define bswap64(x) _byteswap_uint64(x) #endif -#ifdef bswap32 +#if defined(bswap32) #undef ntohl #undef htonl #define ntohl(x) bswap32(x) #define htonl(x) bswap32(x) -#ifndef __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 -# else -# error "Cannot determine endianness" -# endif +#endif + +#if defined(bswap64) + +#undef ntohll +#undef htonll +#define ntohll(x) bswap64(x) +#define htonll(x) bswap64(x) + +#else + +#undef ntohll +#undef htonll + +#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 + +#if !defined(__BYTE_ORDER) +# error "Cannot determine endianness" #endif #if __BYTE_ORDER == __BIG_ENDIAN # define ntohll(n) (n) # define htonll(n)
Re: htonll, ntohll
On 2013-11-05 01.00, Ramsay Jones wrote: > On 31/10/13 13:24, Torsten Bögershausen wrote: >> On 2013-10-30 22.07, Ramsay Jones wrote: > [ ... ] >>> Yep, this was the first thing I did as well! ;-) (*late* last night) >>> >>> I haven't had time today to look into fixing up the msvc build >>> (or a complete re-write), so I look forward to seeing your solution. >>> (do you have msvc available? - or do you want me to look at fixing >>> it? maybe in a day or two?) >>> >> Ramsay, >> I don't have msvc, so feel free to go ahead, as much as you can. >> >> I'll send a patch for the test code I have made, and put bswap.h on hold for >> a week >> (to be able to continue with t5601/connect.c) > > Unfortunately, I haven't had much time to look into this. > > I do have a patch (given below) that works on Linux, cygwin, > MinGW and msvc. However, the msvc build is still broken (as a > result of _other_ commits in this 'jk/pack-bitmap' branch; as > well as the use of a VLA in another commit). > > So, I still have work to do! :( > > Anyway, I thought I would send what I have, so you can take a look. > Note, that I don't have an big-endian machine to test this on, so > YMMV. Indeed, the *only* testing I have done is to run the test added > by this branch (t5310-pack-bitmaps.sh), which works on Linux, cygwin > and MinGW. > > [Note: I have never particularly liked htons, htonl et.al., so adding > these htonll/ntohll functions doesn't thrill me! :-D For example see > this post[1], which echo's my sentiments exactly.] > > HTH > > ATB, > Ramsay Jones > > [1] http://commandcenter.blogspot.co.uk/2012/04/byte-order-fallacy.html > > -- >8 -- > Subject: [PATCH] compat/bswap.h: Fix build on cygwin, MinGW and msvc > > --- > compat/bswap.h | 97 > -- > 1 file changed, 68 insertions(+), 29 deletions(-) > > diff --git a/compat/bswap.h b/compat/bswap.h > index ea1a9ed..c18a78e 100644 > --- a/compat/bswap.h > +++ b/compat/bswap.h > @@ -17,7 +17,20 @@ static inline uint32_t default_swab32(uint32_t val) > ((val & 0x00ff) << 24)); > } > > +static inline uint64_t default_bswap64(uint64_t val) > +{ > + return (((val & (uint64_t)0x00ffULL) << 56) | > + ((val & (uint64_t)0xff00ULL) << 40) | > + ((val & (uint64_t)0x00ffULL) << 24) | > + ((val & (uint64_t)0xff00ULL) << 8) | > + ((val & (uint64_t)0x00ffULL) >> 8) | > + ((val & (uint64_t)0xff00ULL) >> 24) | > + ((val & (uint64_t)0x00ffULL) >> 40) | > + ((val & (uint64_t)0xff00ULL) >> 56)); > +} > + > #undef bswap32 > +#undef bswap64 > > #if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__)) > > @@ -32,54 +45,80 @@ static inline uint32_t git_bswap32(uint32_t x) > return result; > } > > +#define bswap64 git_bswap64 > +#if defined(__x86_64__) > +static inline uint64_t git_bswap64(uint64_t x) > +{ > + uint64_t result; > + if (__builtin_constant_p(x)) > + result = default_bswap64(x); > + else > + __asm__("bswap %q0" : "=r" (result) : "0" (x)); > + return result; > +} > +#else > +static inline uint64_t git_bswap64(uint64_t x) > +{ > + union { uint64_t i64; uint32_t i32[2]; } tmp, result; > + if (__builtin_constant_p(x)) > + result.i64 = default_bswap64(x); > + else { > + tmp.i64 = x; > + result.i32[0] = git_bswap32(tmp.i32[1]); > + result.i32[1] = git_bswap32(tmp.i32[0]); > + } > + return result.i64; > +} > +#endif > + > #elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64)) > > #include > > #define bswap32(x) _byteswap_ulong(x) > +#define bswap64(x) _byteswap_uint64(x) > > #endif > > -#ifdef bswap32 > +#if defined(bswap32) > > #undef ntohl > #undef htonl > #define ntohl(x) bswap32(x) > #define htonl(x) bswap32(x) > > -#ifndef __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 > -#else > -#error "Cannot determine endian
Re: htonll, ntohll
W dniu 2013-11-05 01:00, Ramsay Jones pisze: [Note: I have never particularly liked htons, htonl et.al., so adding these htonll/ntohll functions doesn't thrill me! :-D For example see this post[1], which echo's my sentiments exactly.] HTH ATB, Ramsay Jones [1] http://commandcenter.blogspot.co.uk/2012/04/byte-order-fallacy.html Errr... htonl is about host to network order, and not about big- or little-endianness of architecture. The macros are good, its their implementation that might fail [1]. -- Jakub Narębski -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: htonll, ntohll
On 2013-11-05 01:00, Ramsay Jones wrote: [Note: I have never particularly liked htons, htonl et.al., so adding these htonll/ntohll functions doesn't thrill me! :-D For example see this post[1], which echo's my sentiments exactly.] That post actually contradicts your statement, as it clearly states that "someone at Adobe figured out about byte order and there would have been no problems transferring files between (big-endian and little-endian) machines... if the people at Adobe wrote proper code to encode and decode their files". htonl(), ntohl(), htons(), ntohs() are those "encode" and "decode" functions. If you or the author of the post you linked think otherwise, you're misinformed and need to learn what encoding and decoding means. -- Andreas Ericsson andreas.erics...@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 Considering the successes of the wars on alcohol, poverty, drugs and terror, I think we should give some serious thought to declaring war on peace. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Test code for htonll, ntohll
test-endianess.c adds test code for htonl(), ntohll() and the recently introduced ntohll() and htonll() The test is called in t0070 Signed-off-by: Torsten Bögershausen --- .gitignore |1 + Makefile |3 +++ t/t0070-fundamental.sh |3 +++ test-endianess.c | 58 4 files changed, 65 insertions(+) create mode 100644 test-endianess.c diff --git a/.gitignore b/.gitignore index 66199ed..750e7d8 100644 --- a/.gitignore +++ b/.gitignore @@ -183,6 +183,7 @@ /test-date /test-delta /test-dump-cache-tree +/test-endianess /test-scrap-cache-tree /test-genrandom /test-index-version diff --git a/Makefile b/Makefile index 07b0626..50957c7 100644 --- a/Makefile +++ b/Makefile @@ -555,6 +555,7 @@ TEST_PROGRAMS_NEED_X += test-ctype TEST_PROGRAMS_NEED_X += test-date TEST_PROGRAMS_NEED_X += test-delta TEST_PROGRAMS_NEED_X += test-dump-cache-tree +TEST_PROGRAMS_NEED_X += test-endianess TEST_PROGRAMS_NEED_X += test-genrandom TEST_PROGRAMS_NEED_X += test-hashmap TEST_PROGRAMS_NEED_X += test-index-version @@ -2275,6 +2276,8 @@ test-date$X: date.o ctype.o test-delta$X: diff-delta.o patch-delta.o +test-endianess$X: test-endianess.c + test-line-buffer$X: vcs-svn/lib.a test-parse-options$X: parse-options.o parse-options-cb.o diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh index 5ed69a6..dc687da 100755 --- a/t/t0070-fundamental.sh +++ b/t/t0070-fundamental.sh @@ -33,5 +33,8 @@ test_expect_success 'check for a bug in the regex routines' ' # if this test fails, re-build git with NO_REGEX=1 test-regex ' +test_expect_success 'test endianess, htonll(), ntohll()' ' + test-endianess +' test_done diff --git a/test-endianess.c b/test-endianess.c new file mode 100644 index 000..5b49175 --- /dev/null +++ b/test-endianess.c @@ -0,0 +1,58 @@ +#include "cache.h" + +int main(int argc, char **argv) +{ + union { + uint8_t bytes[8]; + uint32_t word32; + uint64_t word64; + } wordll; + volatile uint64_t word64; + volatile uint32_t word32; + + wordll.bytes[0] = 0x80; + wordll.bytes[1] = 0x81; + wordll.bytes[2] = 0x82; + wordll.bytes[3] = 0x83; + wordll.bytes[4] = 0x00; + wordll.bytes[5] = 0x01; + wordll.bytes[6] = 0x02; + wordll.bytes[7] = 0x03; + + word32 = htonl(wordll.word32); + if (word32 != 0x80818283) + die("htonl word32 != 0x80818283"); + + word64 = htonll(wordll.word64); + if (word64 != 0x8081828300010203ULL) + die("htonll word64 != 0x8081828300010203"); + + word32 = ntohl(wordll.word32); + if (word32 != 0x80818283) + die("ntohl word32 != 0x80818283"); + + word64 = ntohll(wordll.word64); + if (word64 != 0x8081828300010203ULL) + die("ntohll word64 != 0x8081828300010203"); + + wordll.bytes[0] = 0x04; + wordll.bytes[4] = 0x84; + + word32 = htonl(wordll.word32); + if (word32 != 0x04818283) + die("htonl word32 != 0x04818283"); + + word64 = htonll(wordll.word64); + if (word64 != 0x0481828384010203ULL) + die("htonll word64 != 0x0481828384010203"); + + word32 = ntohl(wordll.word32); + if (word32 != 0x04818283) + die("ntohl word32 != 0x04818283"); + + word64 = ntohll(wordll.word64); + if (word64 != 0x0481828384010203ULL) + die("ntohll word64 != 0x0481828384010203"); + + return 0; +} -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html