Re: htonll, ntohll

["Torsten Bögershausen" <tboegi@web.de>]

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 <tboegi@web.de> 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 <byteswap.h>
>>> header file (which had already been included by git-compat-util.h).
>>> In order to suppress the error, ensure that the <byteswap.h> header
>>> is included, just like the __GNUC__/__GLIBC__ case, rather than
>>> attempting to define a fallback implementation.
>>>
>>> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
>>> ---
>>>  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 <byteswap.h>
>>>  #	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 <tboegi@web.de>
>> ---
>>  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
>>
>> .
>>
> 
> 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?)
> 
> ATB,
> Ramsay Jones
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)

/Torsten