[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[H. Nikolaus Schaller]

If the host toolchain is not glibc based then the arm kernel build
fails with

 HOSTCC  arch/arm/vdso/vdsomunge
 arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory

Observed with omap2plus_defconfig and compile on Mac OS X with arm ELF
cross-compiler.

Reason: byteswap.h is a glibc only header.

Changed to detect the host and include the right file as described at:

 https://bugs.freedesktop.org/show_bug.cgi?id=8882

Tested to compile on Mac OS X 10.9.5 host.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
arch/arm/vdso/vdsomunge.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
index aedec81..5364cea 100644
--- a/arch/arm/vdso/vdsomunge.c
+++ b/arch/arm/vdso/vdsomunge.c
@@ -45,7 +45,25 @@
 * it does.
 */

+#if defined(__linux__)
#include <byteswap.h>
+#elif defined(__OpenBSD__)
+#include <sys/endian.h>
+#define bswap_16 __swap16
+#define bswap_32 __swap32
+#define bswap_64 __swap64
+#elif defined(__APPLE__)
+#include <libkern/OSByteOrder.h>
+#define bswap_16 OSSwapInt16
+#define bswap_32 OSSwapInt32
+#define bswap_64 OSSwapInt64
+#else
+#include <sys/endian.h>
+#define bswap_16 bswap16
+#define bswap_32 bswap32
+#define bswap_64 bswap64
+#endif
+
#include <elf.h>
#include <errno.h>
#include <fcntl.h>
-- 
2.5.1

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[Ard Biesheuvel]

On 30 September 2015 at 17:56, H. Nikolaus Schaller <hns@goldelico.com> wrote:
> If the host toolchain is not glibc based then the arm kernel build
> fails with
>
>  HOSTCC  arch/arm/vdso/vdsomunge
>  arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory
>
> Observed with omap2plus_defconfig and compile on Mac OS X with arm ELF
> cross-compiler.
>
> Reason: byteswap.h is a glibc only header.
>
> Changed to detect the host and include the right file as described at:
>
>  https://bugs.freedesktop.org/show_bug.cgi?id=8882
>
> Tested to compile on Mac OS X 10.9.5 host.
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> arch/arm/vdso/vdsomunge.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
> index aedec81..5364cea 100644
> --- a/arch/arm/vdso/vdsomunge.c
> +++ b/arch/arm/vdso/vdsomunge.c
> @@ -45,7 +45,25 @@
>  * it does.
>  */
>
> +#if defined(__linux__)
> #include <byteswap.h>
> +#elif defined(__OpenBSD__)
> +#include <sys/endian.h>
> +#define bswap_16 __swap16
> +#define bswap_32 __swap32
> +#define bswap_64 __swap64
> +#elif defined(__APPLE__)
> +#include <libkern/OSByteOrder.h>
> +#define bswap_16 OSSwapInt16
> +#define bswap_32 OSSwapInt32
> +#define bswap_64 OSSwapInt64
> +#else
> +#include <sys/endian.h>
> +#define bswap_16 bswap16
> +#define bswap_32 bswap32
> +#define bswap_64 bswap64
> +#endif
> +

Have you tried this?

#define bswap_16 __builtin_bswap16
#define bswap_32 __builtin_bswap32
#define bswap_64 __builtin_bswap64

https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[H. Nikolaus Schaller]

Am 30.09.2015 um 18:02 schrieb Ard Biesheuvel <ard.biesheuvel@linaro.org>:

> On 30 September 2015 at 17:56, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> If the host toolchain is not glibc based then the arm kernel build
>> fails with
>> 
>> HOSTCC  arch/arm/vdso/vdsomunge
>> arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory
>> 
>> Observed with omap2plus_defconfig and compile on Mac OS X with arm ELF
>> cross-compiler.
>> 
>> Reason: byteswap.h is a glibc only header.
>> 
>> Changed to detect the host and include the right file as described at:
>> 
>> https://bugs.freedesktop.org/show_bug.cgi?id=8882
>> 
>> Tested to compile on Mac OS X 10.9.5 host.
>> 
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> arch/arm/vdso/vdsomunge.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>> 
>> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
>> index aedec81..5364cea 100644
>> --- a/arch/arm/vdso/vdsomunge.c
>> +++ b/arch/arm/vdso/vdsomunge.c
>> @@ -45,7 +45,25 @@
>> * it does.
>> */
>> 
>> +#if defined(__linux__)
>> #include <byteswap.h>
>> +#elif defined(__OpenBSD__)
>> +#include <sys/endian.h>
>> +#define bswap_16 __swap16
>> +#define bswap_32 __swap32
>> +#define bswap_64 __swap64
>> +#elif defined(__APPLE__)
>> +#include <libkern/OSByteOrder.h>
>> +#define bswap_16 OSSwapInt16
>> +#define bswap_32 OSSwapInt32
>> +#define bswap_64 OSSwapInt64
>> +#else
>> +#include <sys/endian.h>
>> +#define bswap_16 bswap16
>> +#define bswap_32 bswap32
>> +#define bswap_64 bswap64
>> +#endif
>> +
> 
> Have you tried this?
> 
> #define bswap_16 __builtin_bswap16
> #define bswap_32 __builtin_bswap32
> #define bswap_64 __builtin_bswap64
> 
> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

OS X host uses llvm and I am not sure if these builtins are
always available.

So it replaces an include file dependency by a compiler
built-in dependency. IMHO my approach is more general
and should cover a broader range of compile hosts.

This is a tool compiled by HOSTCC to be run on the build
host - so I think we can't assume to have a gcc host compiler
(while for the CC cross-compiler we can).

Thanks,
Nikolaus Schaller

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[Ard Biesheuvel]

On 30 September 2015 at 18:13, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>
> Am 30.09.2015 um 18:02 schrieb Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>
>> On 30 September 2015 at 17:56, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> If the host toolchain is not glibc based then the arm kernel build
>>> fails with
>>>
>>> HOSTCC  arch/arm/vdso/vdsomunge
>>> arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory
>>>
>>> Observed with omap2plus_defconfig and compile on Mac OS X with arm ELF
>>> cross-compiler.
>>>
>>> Reason: byteswap.h is a glibc only header.
>>>
>>> Changed to detect the host and include the right file as described at:
>>>
>>> https://bugs.freedesktop.org/show_bug.cgi?id=8882
>>>
>>> Tested to compile on Mac OS X 10.9.5 host.
>>>
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>> ---
>>> arch/arm/vdso/vdsomunge.c | 18 ++++++++++++++++++
>>> 1 file changed, 18 insertions(+)
>>>
>>> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
>>> index aedec81..5364cea 100644
>>> --- a/arch/arm/vdso/vdsomunge.c
>>> +++ b/arch/arm/vdso/vdsomunge.c
>>> @@ -45,7 +45,25 @@
>>> * it does.
>>> */
>>>
>>> +#if defined(__linux__)
>>> #include <byteswap.h>
>>> +#elif defined(__OpenBSD__)
>>> +#include <sys/endian.h>
>>> +#define bswap_16 __swap16
>>> +#define bswap_32 __swap32
>>> +#define bswap_64 __swap64
>>> +#elif defined(__APPLE__)
>>> +#include <libkern/OSByteOrder.h>
>>> +#define bswap_16 OSSwapInt16
>>> +#define bswap_32 OSSwapInt32
>>> +#define bswap_64 OSSwapInt64
>>> +#else
>>> +#include <sys/endian.h>
>>> +#define bswap_16 bswap16
>>> +#define bswap_32 bswap32
>>> +#define bswap_64 bswap64
>>> +#endif
>>> +
>>
>> Have you tried this?
>>
>> #define bswap_16 __builtin_bswap16
>> #define bswap_32 __builtin_bswap32
>> #define bswap_64 __builtin_bswap64
>>
>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
>
> OS X host uses llvm and I am not sure if these builtins are
> always available.
>

I am pretty sure recent clang supports these as well. Could you please try it?

> So it replaces an include file dependency by a compiler
> built-in dependency. IMHO my approach is more general
> and should cover a broader range of compile hosts.
>
> This is a tool compiled by HOSTCC to be run on the build
> host - so I think we can't assume to have a gcc host compiler
> (while for the CC cross-compiler we can).
>

If my suggestion works, we'll support host side GCC and clang without
decorating the source code with lots of #ifdefs for OSes few people
care about. That would be an improvement imo

Thanks,
Ard.

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[H. Nikolaus Schaller]

Hi,

Am 30.09.2015 um 18:17 schrieb Ard Biesheuvel <ard.biesheuvel@linaro.org>:

> On 30 September 2015 at 18:13, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>> Am 30.09.2015 um 18:02 schrieb Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> 
>>> On 30 September 2015 at 17:56, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>>> If the host toolchain is not glibc based then the arm kernel build
>>>> fails with
>>>> 
>>>> HOSTCC  arch/arm/vdso/vdsomunge
>>>> arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory
>>>> 
>>>> Observed with omap2plus_defconfig and compile on Mac OS X with arm ELF
>>>> cross-compiler.
>>>> 
>>>> Reason: byteswap.h is a glibc only header.
>>>> 
>>>> Changed to detect the host and include the right file as described at:
>>>> 
>>>> https://bugs.freedesktop.org/show_bug.cgi?id=8882
>>>> 
>>>> Tested to compile on Mac OS X 10.9.5 host.
>>>> 
>>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>>> ---
>>>> arch/arm/vdso/vdsomunge.c | 18 ++++++++++++++++++
>>>> 1 file changed, 18 insertions(+)
>>>> 
>>>> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
>>>> index aedec81..5364cea 100644
>>>> --- a/arch/arm/vdso/vdsomunge.c
>>>> +++ b/arch/arm/vdso/vdsomunge.c
>>>> @@ -45,7 +45,25 @@
>>>> * it does.
>>>> */
>>>> 
>>>> +#if defined(__linux__)
>>>> #include <byteswap.h>
>>>> +#elif defined(__OpenBSD__)
>>>> +#include <sys/endian.h>
>>>> +#define bswap_16 __swap16
>>>> +#define bswap_32 __swap32
>>>> +#define bswap_64 __swap64
>>>> +#elif defined(__APPLE__)
>>>> +#include <libkern/OSByteOrder.h>
>>>> +#define bswap_16 OSSwapInt16
>>>> +#define bswap_32 OSSwapInt32
>>>> +#define bswap_64 OSSwapInt64
>>>> +#else
>>>> +#include <sys/endian.h>
>>>> +#define bswap_16 bswap16
>>>> +#define bswap_32 bswap32
>>>> +#define bswap_64 bswap64
>>>> +#endif
>>>> +
>>> 
>>> Have you tried this?
>>> 
>>> #define bswap_16 __builtin_bswap16
>>> #define bswap_32 __builtin_bswap32
>>> #define bswap_64 __builtin_bswap64
>>> 
>>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
>> 
>> OS X host uses llvm and I am not sure if these builtins are
>> always available.
>> 
> 
> I am pretty sure recent clang supports these as well. Could you please try it?

What release date (= feature set) of a compiler would you no longer consider as "recent"?

> 
>> So it replaces an include file dependency by a compiler
>> built-in dependency. IMHO my approach is more general
>> and should cover a broader range of compile hosts.
>> 
>> This is a tool compiled by HOSTCC to be run on the build
>> host - so I think we can't assume to have a gcc host compiler
>> (while for the CC cross-compiler we can).
>> 
> 
> If my suggestion works, we'll support host side GCC and clang without
> decorating the source code with lots of #ifdefs for OSes few people
> care about. That would be an improvement imo

Indeed it would be the best solution.

But at least my Apple LLVM version 6.0 (clang-600.0.57) (based on LLVM 3.5svn)
it does *not* work (see also for example http://sourceforge.net/p/flac/bugs/416/ ).

Clang 3.5 appears to provide all features of GCC 4.7.

I have checked and it appears that __builtin_bswap16 only became available in
GCC 4.8 (released 2013-03-22). And maybe in Clang 3.7.

Unfortunately I could not find a reference what the minimum required gcc for
the Linux HOSTCC currently is.

So if we require gcc >= 4.8 just by this small tool, this should be made well
known and well decided.

Therefore I would propose to use the #ifdef approach now and revise 5 years
after gcc 4.8 release (2013 ==> 2018) if __builtin_bswap16 (or something else)
has become a widely available standard.

I can add this to the patch commit V2 for reference.

BR and thanks,
Nikolaus Schaller

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[Nathan Lynch]

On 09/30/2015 11:17 AM, Ard Biesheuvel wrote:
> On 30 September 2015 at 18:13, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>
>> Am 30.09.2015 um 18:02 schrieb Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>
>>> Have you tried this?
>>>
>>> #define bswap_16 __builtin_bswap16
>>> #define bswap_32 __builtin_bswap32
>>> #define bswap_64 __builtin_bswap64
>>>
>>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
>>
>> OS X host uses llvm and I am not sure if these builtins are
>> always available.
>>
> 
> I am pretty sure recent clang supports these as well. Could you please try it?

Well, I think GCC did not provide __builtin_bswap16 consistently until
the 4.8 release:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52624

That seems too recent to me.

vdsomunge makes only three or four potentially byteswapped accesses to
the ELF header.  It's not worth a lot of effort to try to use the most
optimal implementation available.  Why not just use a generic
implementation like is found in mips' elf2ecoff?

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[H. Nikolaus Schaller]

Am 30.09.2015 um 19:37 schrieb Nathan Lynch <Nathan_Lynch@mentor.com>:

> On 09/30/2015 11:17 AM, Ard Biesheuvel wrote:
>> On 30 September 2015 at 18:13, H. Nikolaus Schaller <hns@goldelico.com> wrote:
>>> 
>>> Am 30.09.2015 um 18:02 schrieb Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>> 
>>>> Have you tried this?
>>>> 
>>>> #define bswap_16 __builtin_bswap16
>>>> #define bswap_32 __builtin_bswap32
>>>> #define bswap_64 __builtin_bswap64
>>>> 
>>>> https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html
>>> 
>>> OS X host uses llvm and I am not sure if these builtins are
>>> always available.
>>> 
>> 
>> I am pretty sure recent clang supports these as well. Could you please try it?
> 
> Well, I think GCC did not provide __builtin_bswap16 consistently until
> the 4.8 release:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52624
> 
> That seems too recent to me.

Same for me.

> 
> vdsomunge makes only three or four potentially byteswapped accesses to
> the ELF header.  It's not worth a lot of effort to try to use the most
> optimal implementation available.

Especially as it does not run on the target device but the build host.

>  Why not just use a generic
> implementation like is found in mips' elf2ecoff?

Do you have a reference?
I can't find byte swapping in

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/mips/boot/elf2ecoff.c?id=refs/tags/v4.3-rc3

BR and thanks,
Nikolaus Schaller

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[Nathan Lynch]

On 09/30/2015 12:47 PM, H. Nikolaus Schaller wrote:
> 
> Am 30.09.2015 um 19:37 schrieb Nathan Lynch <Nathan_Lynch@mentor.com>:
>>  Why not just use a generic
>> implementation like is found in mips' elf2ecoff?
> 
> Do you have a reference?
> I can't find byte swapping in
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/mips/boot/elf2ecoff.c?id=refs/tags/v4.3-rc3

See the swab16 and swab32 macros (yes, "swab" not "swap").

Or the __constant_swab* macros in include/linux/uapi/swab.h.

[PATCH] ARM: fix vdsomunge depends on glibc specific byteswap.h

[H. Nikolaus Schaller]

Am 30.09.2015 um 20:17 schrieb Nathan Lynch <Nathan_Lynch@mentor.com>:

> On 09/30/2015 12:47 PM, H. Nikolaus Schaller wrote:
>> 
>> Am 30.09.2015 um 19:37 schrieb Nathan Lynch <Nathan_Lynch@mentor.com>:
>>> Why not just use a generic
>>> implementation like is found in mips' elf2ecoff?
>> 
>> Do you have a reference?
>> I can't find byte swapping in
>> 
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/mips/boot/elf2ecoff.c?id=refs/tags/v4.3-rc3
> 
> See the swab16 and swab32 macros (yes, "swab" not "swap").
> 
> Or the __constant_swab* macros in include/linux/uapi/swab.h.
> 

Ah, I did search for swaP...

And, I though that they are more conditional on the CPU endianness but that
would be sort of htons() and ntohl() and friends.

Then I see no problem adding such macros and getting rid of the #include
completely. Which is compiler independent and OS independent.

Will prepare and submit a V2 asap.

Thanks and BR,
Nikolaus

[PATCH] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

If the host toolchain is not glibc based then the arm kernel build
fails with

 HOSTCC  arch/arm/vdso/vdsomunge
 arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory

Observed: with omap2plus_defconfig and compile on Mac OS X with arm ELF
cross-compiler.

Reason: byteswap.h is a glibc only header.

Solution: replace by private byte-swapping macros (taken from
arch/mips/boot/elf2ecoff.c)

Tested to compile on Mac OS X 10.9.5 host.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
arch/arm/vdso/vdsomunge.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
index aedec81..27a9a0b 100644
--- a/arch/arm/vdso/vdsomunge.c
+++ b/arch/arm/vdso/vdsomunge.c
@@ -45,7 +45,18 @@
 * it does.
 */

-#include <byteswap.h>
+#define swab16(x) \
+	((unsigned short)( \
+		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
+		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
+
+#define swab32(x) \
+	((unsigned int)( \
+		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
+		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
+		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
+		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
+
#include <elf.h>
#include <errno.h>
#include <fcntl.h>
@@ -104,17 +115,17 @@ static void cleanup(void)

static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
{
-	return swap ? bswap_32(word) : word;
+	return swap ? swab32(word) : word;
}

static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
{
-	return swap ? bswap_16(half) : half;
+	return swap ? swab16(half) : half;
}

static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
{
-	*dst = swap ? bswap_32(val) : val;
+	*dst = swap ? swab32(val) : val;
}

int main(int argc, char **argv)
-- 
2.5.1

[PATCH] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

If the host toolchain is not glibc based then the arm kernel build
fails with

HOSTCC  arch/arm/vdso/vdsomunge
arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory

Observed: with omap2plus_defconfig and compile on Mac OS X with arm ELF
cross-compiler.

Reason: byteswap.h is a glibc only header.

Solution: replace by private byte-swapping macros (taken from
arch/mips/boot/elf2ecoff.c)

Tested to compile on Mac OS X 10.9.5 host.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
arch/arm/vdso/vdsomunge.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
index aedec81..27a9a0b 100644
--- a/arch/arm/vdso/vdsomunge.c
+++ b/arch/arm/vdso/vdsomunge.c
@@ -45,7 +45,18 @@
* it does.
*/

-#include <byteswap.h>
+#define swab16(x) \
+	((unsigned short)( \
+		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
+		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
+
+#define swab32(x) \
+	((unsigned int)( \
+		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
+		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
+		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
+		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
+
#include <elf.h>
#include <errno.h>
#include <fcntl.h>
@@ -104,17 +115,17 @@ static void cleanup(void)

static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
{
-	return swap ? bswap_32(word) : word;
+	return swap ? swab32(word) : word;
}

static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
{
-	return swap ? bswap_16(half) : half;
+	return swap ? swab16(half) : half;
}

static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
{
-	*dst = swap ? bswap_32(val) : val;
+	*dst = swap ? swab32(val) : val;
}

int main(int argc, char **argv)
-- 
2.5.1

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

If the host toolchain is not glibc based then the arm kernel build
fails with

 HOSTCC  arch/arm/vdso/vdsomunge
 arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory

Observed: with omap2plus_defconfig and compile on Mac OS X with arm ELF
cross-compiler.

Reason: byteswap.h is a glibc only header.

Solution: replace by private byte-swapping macros (taken from
arch/mips/boot/elf2ecoff.c)

Tested to compile on Mac OS X 10.9.5 host.

Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
arch/arm/vdso/vdsomunge.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
index aedec81..27a9a0b 100644
--- a/arch/arm/vdso/vdsomunge.c
+++ b/arch/arm/vdso/vdsomunge.c
@@ -45,7 +45,18 @@
 * it does.
 */

-#include <byteswap.h>
+#define swab16(x) \
+	((unsigned short)( \
+		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
+		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
+
+#define swab32(x) \
+	((unsigned int)( \
+		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
+		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
+		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
+		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
+
#include <elf.h>
#include <errno.h>
#include <fcntl.h>
@@ -104,17 +115,17 @@ static void cleanup(void)

static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
{
-	return swap ? bswap_32(word) : word;
+	return swap ? swab32(word) : word;
}

static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
{
-	return swap ? bswap_16(half) : half;
+	return swap ? swab16(half) : half;
}

static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
{
-	*dst = swap ? bswap_32(val) : val;
+	*dst = swap ? swab32(val) : val;
}

int main(int argc, char **argv)
-- 
2.5.1

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

ping.

Am 03.10.2015 um 22:46 schrieb H. Nikolaus Schaller <hns@goldelico.com>:

> If the host toolchain is not glibc based then the arm kernel build
> fails with
> 
> HOSTCC  arch/arm/vdso/vdsomunge
> arch/arm/vdso/vdsomunge.c:48:22: fatal error: byteswap.h: No such file or directory
> 
> Observed: with omap2plus_defconfig and compile on Mac OS X with arm ELF
> cross-compiler.
> 
> Reason: byteswap.h is a glibc only header.
> 
> Solution: replace by private byte-swapping macros (taken from
> arch/mips/boot/elf2ecoff.c)
> 
> Tested to compile on Mac OS X 10.9.5 host.
> 
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> arch/arm/vdso/vdsomunge.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
> index aedec81..27a9a0b 100644
> --- a/arch/arm/vdso/vdsomunge.c
> +++ b/arch/arm/vdso/vdsomunge.c
> @@ -45,7 +45,18 @@
> * it does.
> */
> 
> -#include <byteswap.h>
> +#define swab16(x) \
> +	((unsigned short)( \
> +		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
> +
> +#define swab32(x) \
> +	((unsigned int)( \
> +		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
> +		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
> +		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
> +
> #include <elf.h>
> #include <errno.h>
> #include <fcntl.h>
> @@ -104,17 +115,17 @@ static void cleanup(void)
> 
> static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
> {
> -	return swap ? bswap_32(word) : word;
> +	return swap ? swab32(word) : word;
> }
> 
> static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
> {
> -	return swap ? bswap_16(half) : half;
> +	return swap ? swab16(half) : half;
> }
> 
> static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
> {
> -	*dst = swap ? bswap_32(val) : val;
> +	*dst = swap ? swab32(val) : val;
> }
> 
> int main(int argc, char **argv)
> -- 
> 2.5.1
> 

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[Nathan Lynch]

On 10/14/2015 07:47 AM, H. Nikolaus Schaller wrote:
>> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
>> index aedec81..27a9a0b 100644
>> --- a/arch/arm/vdso/vdsomunge.c
>> +++ b/arch/arm/vdso/vdsomunge.c
>> @@ -45,7 +45,18 @@
>> * it does.
>> */
>>
>> -#include <byteswap.h>
>> +#define swab16(x) \
>> +	((unsigned short)( \
>> +		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
>> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
>> +
>> +#define swab32(x) \
>> +	((unsigned int)( \
>> +		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
>> +		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
>> +		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
>> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
>> +
>> #include <elf.h>
>> #include <errno.h>
>> #include <fcntl.h>
>> @@ -104,17 +115,17 @@ static void cleanup(void)
>>
>> static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
>> {
>> -	return swap ? bswap_32(word) : word;
>> +	return swap ? swab32(word) : word;
>> }
>>
>> static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
>> {
>> -	return swap ? bswap_16(half) : half;
>> +	return swap ? swab16(half) : half;
>> }
>>
>> static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
>> {
>> -	*dst = swap ? bswap_32(val) : val;
>> +	*dst = swap ? swab32(val) : val;
>> }
> ping.

Sorry for the delay.

This is okay but I'd prefer the swab macros to be below the #include
lines, and it would be easier for me to deal with a patch that isn't
whitespace-damaged.

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

Am 14.10.2015 um 16:16 schrieb Nathan Lynch <nathan_lynch@mentor.com>:

> On 10/14/2015 07:47 AM, H. Nikolaus Schaller wrote:
>>> diff --git a/arch/arm/vdso/vdsomunge.c b/arch/arm/vdso/vdsomunge.c
>>> index aedec81..27a9a0b 100644
>>> --- a/arch/arm/vdso/vdsomunge.c
>>> +++ b/arch/arm/vdso/vdsomunge.c
>>> @@ -45,7 +45,18 @@
>>> * it does.
>>> */
>>> 
>>> -#include <byteswap.h>
>>> +#define swab16(x) \
>>> +	((unsigned short)( \
>>> +		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
>>> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
>>> +
>>> +#define swab32(x) \
>>> +	((unsigned int)( \
>>> +		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
>>> +		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
>>> +		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
>>> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
>>> +
>>> #include <elf.h>
>>> #include <errno.h>
>>> #include <fcntl.h>
>>> @@ -104,17 +115,17 @@ static void cleanup(void)
>>> 
>>> static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
>>> {
>>> -	return swap ? bswap_32(word) : word;
>>> +	return swap ? swab32(word) : word;
>>> }
>>> 
>>> static Elf32_Half read_elf_half(Elf32_Half half, bool swap)
>>> {
>>> -	return swap ? bswap_16(half) : half;
>>> +	return swap ? swab16(half) : half;
>>> }
>>> 
>>> static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
>>> {
>>> -	*dst = swap ? bswap_32(val) : val;
>>> +	*dst = swap ? swab32(val) : val;
>>> }
>> ping.
> 
> Sorry for the delay.
> 
> This is okay but I'd prefer the swab macros to be below the #include
> lines,

Ok.

> and it would be easier for me to deal with a patch that isn't
> whitespace-damaged.

You mean:

ERROR: space prohibited before that close parenthesis ')'
#46: FILE: arch/arm/vdso/vdsomunge.c:64:
+		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))

ERROR: space prohibited before that close parenthesis ')'
#53: FILE: arch/arm/vdso/vdsomunge.c:71:
+		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))

Well, I had copied that verbatim from arch/mips/boot/elf2ecoff.c and
thought that it is better readable, but it is easy to fix.

V3 is coming.

BR and thanks,
Nikolaus

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[Russell King - ARM Linux]

On Thu, Oct 15, 2015 at 07:52:38AM +0200, H. Nikolaus Schaller wrote:
> ERROR: space prohibited before that close parenthesis ')'
> #46: FILE: arch/arm/vdso/vdsomunge.c:64:
> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))

I have a pet hatred of too many parens.  I also have a hatred of idiotic
casts.  What about changing the above to one of:

		(((x) & 0xff00) >> 8)
		(((x) >> 8) & 0xff)

> 
> ERROR: space prohibited before that close parenthesis ')'
> #53: FILE: arch/arm/vdso/vdsomunge.c:71:
> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))

and one of:

		(((x) & 0xff000000) >> 24)
		(((x) >> 24) & 0xff)

?

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

Am 15.10.2015 um 18:37 schrieb Russell King - ARM Linux <linux@arm.linux.org.uk>:

> On Thu, Oct 15, 2015 at 07:52:38AM +0200, H. Nikolaus Schaller wrote:
>> ERROR: space prohibited before that close parenthesis ')'
>> #46: FILE: arch/arm/vdso/vdsomunge.c:64:
>> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
> 
> I have a pet hatred of too many parens.  I also have a hatred of idiotic
> casts.  What about changing the above to one of:
> 
> 		(((x) & 0xff00) >> 8)
> 		(((x) >> 8) & 0xff)
> 
>> 
>> ERROR: space prohibited before that close parenthesis ')'
>> #53: FILE: arch/arm/vdso/vdsomunge.c:71:
>> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
> 
> and one of:
> 
> 		(((x) & 0xff000000) >> 24)
> 		(((x) >> 24) & 0xff)
> 
> ?

Well, the semantics is sometimes the same but I think readability goes down
because the macro becomes quite un-symmetric and for >> 8 you should
better be sure that values are unsigned if combining with | operator.

Of course it is questionable why the constants (with U and UL modifiers)
are casted.

The full macro (as copied verbatim from existing arch/mips/boot/elf2ecoff.c) is:

+#define swab16(x) \
+	((unsigned short)( \
+		(((unsigned short)(x) & (unsigned short)0x00ffU) << 8) | \
+		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8)))
+
+#define swab32(x) \
+	((unsigned int)( \
+		(((unsigned int)(x) & (unsigned int)0x000000ffUL) << 24) | \
+		(((unsigned int)(x) & (unsigned int)0x0000ff00UL) <<  8) | \
+		(((unsigned int)(x) & (unsigned int)0x00ff0000UL) >>  8) | \
+		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24)))
+

I have already submitted a V3 of this patch, so please comment that.

A question is why this is not generally available for all HOSTCC based tools
so that we could simply include some header. But since that is no kernel
code but a tool, I think it is not really necessary to put significant efforts
into this piece of code.

This was probably the intention of the original code to simply include
byteswap.h - then you don't even see how complex the macros are -
but it is not a standard header.

BR and thanks,
Nikolaus Schaller

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[Nathan Lynch]

On 10/15/2015 12:52 AM, H. Nikolaus Schaller wrote:
> Am 14.10.2015 um 16:16 schrieb Nathan Lynch <nathan_lynch@mentor.com>:
>> and it would be easier for me to deal with a patch that isn't
>> whitespace-damaged.
> 
> You mean:
> 
> ERROR: space prohibited before that close parenthesis ')'
> #46: FILE: arch/arm/vdso/vdsomunge.c:64:
> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
> 
> ERROR: space prohibited before that close parenthesis ')'
> #53: FILE: arch/arm/vdso/vdsomunge.c:71:
> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
> 
> Well, I had copied that verbatim from arch/mips/boot/elf2ecoff.c and
> thought that it is better readable, but it is easy to fix.

That's not what I was referring to, but it is fine to fix that too.

What I meant was that the first column of the patch body is corrupted.
Your v2 has hunks like this (bad):

static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
{
-	return swap ? bswap_32(word) : word;
+	return swap ? swab32(word) : word;
}


Your v3 has this (good):

 static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
 {
-	*dst = swap ? bswap_32(val) : val;
+	*dst = swap ? swab32(val) : val;
 }

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[H. Nikolaus Schaller]

Am 15.10.2015 um 19:07 schrieb Nathan Lynch <Nathan_Lynch@mentor.com>:

> On 10/15/2015 12:52 AM, H. Nikolaus Schaller wrote:
>> Am 14.10.2015 um 16:16 schrieb Nathan Lynch <nathan_lynch@mentor.com>:
>>> and it would be easier for me to deal with a patch that isn't
>>> whitespace-damaged.
>> 
>> You mean:
>> 
>> ERROR: space prohibited before that close parenthesis ')'
>> #46: FILE: arch/arm/vdso/vdsomunge.c:64:
>> +		(((unsigned short)(x) & (unsigned short)0xff00U) >> 8) ))
>> 
>> ERROR: space prohibited before that close parenthesis ')'
>> #53: FILE: arch/arm/vdso/vdsomunge.c:71:
>> +		(((unsigned int)(x) & (unsigned int)0xff000000UL) >> 24) ))
>> 
>> Well, I had copied that verbatim from arch/mips/boot/elf2ecoff.c and
>> thought that it is better readable, but it is easy to fix.
> 
> That's not what I was referring to, but it is fine to fix that too.
> 
> What I meant was that the first column of the patch body is corrupted.
> Your v2 has hunks like this (bad):
> 
> static Elf32_Word read_elf_word(Elf32_Word word, bool swap)
> {
> -	return swap ? bswap_32(word) : word;
> +	return swap ? swab32(word) : word;
> }
> 
> 
> Your v3 has this (good):
> 
> static void write_elf_word(Elf32_Word val, Elf32_Word *dst, bool swap)
> {
> -	*dst = swap ? bswap_32(val) : val;
> +	*dst = swap ? swab32(val) : val;
> }

Does this mean it is ok now in V3 in all cases? I have switched my patch editor
tool from indirect git imap-send to git send-email (which appears to be more
compatible).

BR and thanks,
Nikolaus Schaller

[PATCH v2] ARM: fix vdsomunge not to depend on glibc specific byteswap.h

[Nathan Lynch]

On 10/15/2015 12:16 PM, H. Nikolaus Schaller wrote:
> Does this mean it is ok now in V3 in all cases? I have switched my patch editor
> tool from indirect git imap-send to git send-email (which appears to be more
> compatible).

v3 applied without issue, so yes, in that respect it is fine.