[PATCH] squashfs: Fix compilation on big endian systems
Signed-off-by: Pali Rohár pali@kernel.org --- fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h>
Hi Pali,
pali@kernel.org wrote on Wed, 6 Apr 2022 23:31:53 +0200:
Would you mind explaining a little bit how this change fixes it? It does not look straightforward to me.
Signed-off-by: Pali Rohár pali@kernel.org
fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h>
Cheers, Miquèl
On Thursday 07 April 2022 09:54:21 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Wed, 6 Apr 2022 23:31:53 +0200:
Would you mind explaining a little bit how this change fixes it? It does not look straightforward to me.
Yes! I though that it is straightforward this change... byteorder/little_endian.h defines cpu_to_le* macros for Little Endian systems and byteorder/big_endian.h for Big Endian systems.
File asm/byteorder.h is then ARCH-specific and implements macros for the current architecture (by including the correct header file).
So currently if you try to compile squashfs for big endian systems you get compile error:
In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs_filesystem.h:11, from fs/squashfs/sqfs_dir.c:16: include/linux/byteorder/big_endian.h:34: warning: "__cpu_to_le32" redefined #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
In file included from fs/squashfs/sqfs_dir.c:10: include/linux/byteorder/little_endian.h:34: note: this is the location of the previous definition #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
Or:
In file included from fs/squashfs/sqfs.c:14: include/linux/byteorder/little_endian.h:89:21: error: redefinition of ‘__be16_to_cpup’ static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~ In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs.c:10: include/linux/byteorder/big_endian.h:89:21: note: previous definition of ‘__be16_to_cpup’ was here static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~
As some header files include correct asm/byteorder.h file and this squashfs includes additional little_endian.h.
Signed-off-by: Pali Rohár pali@kernel.org
fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h>
Cheers, Miquèl
Hi Pali,
pali@kernel.org wrote on Thu, 7 Apr 2022 11:41:59 +0200:
On Thursday 07 April 2022 09:54:21 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Wed, 6 Apr 2022 23:31:53 +0200:
Would you mind explaining a little bit how this change fixes it? It does not look straightforward to me.
Yes! I though that it is straightforward this change... byteorder/little_endian.h defines cpu_to_le* macros for Little Endian systems and byteorder/big_endian.h for Big Endian systems.
File asm/byteorder.h is then ARCH-specific and implements macros for the current architecture (by including the correct header file).
So currently if you try to compile squashfs for big endian systems you get compile error:
In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs_filesystem.h:11, from fs/squashfs/sqfs_dir.c:16: include/linux/byteorder/big_endian.h:34: warning: "__cpu_to_le32" redefined #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
In file included from fs/squashfs/sqfs_dir.c:10: include/linux/byteorder/little_endian.h:34: note: this is the location of the previous definition #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
Or:
In file included from fs/squashfs/sqfs.c:14: include/linux/byteorder/little_endian.h:89:21: error: redefinition of ‘__be16_to_cpup’ static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~ In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs.c:10: include/linux/byteorder/big_endian.h:89:21: note: previous definition of ‘__be16_to_cpup’ was here static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~
As some header files include correct asm/byteorder.h file and this squashfs includes additional little_endian.h.
Great, thanks for the thorough explanation. Based on what you said, wouldn't it be cleaner to just get rid of the little_endian.h include rather than also use the ARCH specific byteorder.h header?
Signed-off-by: Pali Rohár pali@kernel.org
fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h>
Cheers, Miquèl
Thanks, Miquèl
On Thursday 07 April 2022 11:54:55 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Thu, 7 Apr 2022 11:41:59 +0200:
On Thursday 07 April 2022 09:54:21 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Wed, 6 Apr 2022 23:31:53 +0200:
Would you mind explaining a little bit how this change fixes it? It does not look straightforward to me.
Yes! I though that it is straightforward this change... byteorder/little_endian.h defines cpu_to_le* macros for Little Endian systems and byteorder/big_endian.h for Big Endian systems.
File asm/byteorder.h is then ARCH-specific and implements macros for the current architecture (by including the correct header file).
So currently if you try to compile squashfs for big endian systems you get compile error:
In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs_filesystem.h:11, from fs/squashfs/sqfs_dir.c:16: include/linux/byteorder/big_endian.h:34: warning: "__cpu_to_le32" redefined #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
In file included from fs/squashfs/sqfs_dir.c:10: include/linux/byteorder/little_endian.h:34: note: this is the location of the previous definition #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
Or:
In file included from fs/squashfs/sqfs.c:14: include/linux/byteorder/little_endian.h:89:21: error: redefinition of ‘__be16_to_cpup’ static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~ In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs.c:10: include/linux/byteorder/big_endian.h:89:21: note: previous definition of ‘__be16_to_cpup’ was here static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~
As some header files include correct asm/byteorder.h file and this squashfs includes additional little_endian.h.
Great, thanks for the thorough explanation. Based on what you said, wouldn't it be cleaner to just get rid of the little_endian.h include rather than also use the ARCH specific byteorder.h header?
I think that this is not possible. squashfs code uses le16_to_cpu() macro and its correct definition is provided only by arch specific asm/byteorder.h.
Signed-off-by: Pali Rohár pali@kernel.org
fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h> #include <stdlib.h>
Cheers, Miquèl
Thanks, Miquèl
On Thu, Apr 07, 2022 at 11:58:55AM +0200, Pali Rohár wrote:
On Thursday 07 April 2022 11:54:55 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Thu, 7 Apr 2022 11:41:59 +0200:
On Thursday 07 April 2022 09:54:21 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Wed, 6 Apr 2022 23:31:53 +0200:
Would you mind explaining a little bit how this change fixes it? It does not look straightforward to me.
Yes! I though that it is straightforward this change... byteorder/little_endian.h defines cpu_to_le* macros for Little Endian systems and byteorder/big_endian.h for Big Endian systems.
File asm/byteorder.h is then ARCH-specific and implements macros for the current architecture (by including the correct header file).
So currently if you try to compile squashfs for big endian systems you get compile error:
In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs_filesystem.h:11, from fs/squashfs/sqfs_dir.c:16: include/linux/byteorder/big_endian.h:34: warning: "__cpu_to_le32" redefined #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
In file included from fs/squashfs/sqfs_dir.c:10: include/linux/byteorder/little_endian.h:34: note: this is the location of the previous definition #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
Or:
In file included from fs/squashfs/sqfs.c:14: include/linux/byteorder/little_endian.h:89:21: error: redefinition of ‘__be16_to_cpup’ static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~ In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs.c:10: include/linux/byteorder/big_endian.h:89:21: note: previous definition of ‘__be16_to_cpup’ was here static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~
As some header files include correct asm/byteorder.h file and this squashfs includes additional little_endian.h.
Great, thanks for the thorough explanation. Based on what you said, wouldn't it be cleaner to just get rid of the little_endian.h include rather than also use the ARCH specific byteorder.h header?
I think that this is not possible. squashfs code uses le16_to_cpu() macro and its correct definition is provided only by arch specific asm/byteorder.h.
Miquel, is this sufficient explanation? Thanks.
Hi Tom,
trini@konsulko.com wrote on Wed, 1 Jun 2022 10:08:20 -0400:
On Thu, Apr 07, 2022 at 11:58:55AM +0200, Pali Rohár wrote:
On Thursday 07 April 2022 11:54:55 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Thu, 7 Apr 2022 11:41:59 +0200:
On Thursday 07 April 2022 09:54:21 Miquel Raynal wrote:
Hi Pali,
pali@kernel.org wrote on Wed, 6 Apr 2022 23:31:53 +0200:
Would you mind explaining a little bit how this change fixes it? It does not look straightforward to me.
Yes! I though that it is straightforward this change... byteorder/little_endian.h defines cpu_to_le* macros for Little Endian systems and byteorder/big_endian.h for Big Endian systems.
File asm/byteorder.h is then ARCH-specific and implements macros for the current architecture (by including the correct header file).
So currently if you try to compile squashfs for big endian systems you get compile error:
In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs_filesystem.h:11, from fs/squashfs/sqfs_dir.c:16: include/linux/byteorder/big_endian.h:34: warning: "__cpu_to_le32" redefined #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
In file included from fs/squashfs/sqfs_dir.c:10: include/linux/byteorder/little_endian.h:34: note: this is the location of the previous definition #define __cpu_to_le32(x) ((__force __le32)(__u32)(x))
Or:
In file included from fs/squashfs/sqfs.c:14: include/linux/byteorder/little_endian.h:89:21: error: redefinition of ‘__be16_to_cpup’ static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~ In file included from ./arch/powerpc/include/asm/byteorder.h:82, from include/linux/unaligned/access_ok.h:4, from ./arch/powerpc/include/asm/unaligned.h:9, from fs/squashfs/sqfs.c:10: include/linux/byteorder/big_endian.h:89:21: note: previous definition of ‘__be16_to_cpup’ was here static inline __u16 __be16_to_cpup(const __be16 *p) ^~~~~~~~~~~~~~
As some header files include correct asm/byteorder.h file and this squashfs includes additional little_endian.h.
Great, thanks for the thorough explanation. Based on what you said, wouldn't it be cleaner to just get rid of the little_endian.h include rather than also use the ARCH specific byteorder.h header?
I think that this is not possible. squashfs code uses le16_to_cpu() macro and its correct definition is provided only by arch specific asm/byteorder.h.
Miquel, is this sufficient explanation? Thanks.
Yes, sorry, it seems like in U-Boot including the asm header is the way to go, so:
Reviewed-by: Miquel Raynal miquel.raynal@bootlin.com
Thanks, Miquèl
On Wednesday 06 April 2022 23:31:53 Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org
fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h>
#include <stdlib.h>
2.20.1
PING?
On Tuesday 10 May 2022 16:33:56 Pali Rohár wrote:
On Wednesday 06 April 2022 23:31:53 Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org
fs/squashfs/sqfs.c | 3 +-- fs/squashfs/sqfs_dir.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/squashfs/sqfs.c b/fs/squashfs/sqfs.c index 5d9c52af80ba..41cb811c1b32 100644 --- a/fs/squashfs/sqfs.c +++ b/fs/squashfs/sqfs.c @@ -11,8 +11,7 @@ #include <errno.h> #include <fs.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <memalign.h> #include <stdlib.h> #include <string.h> diff --git a/fs/squashfs/sqfs_dir.c b/fs/squashfs/sqfs_dir.c index a265b98fe685..ed83c90682ff 100644 --- a/fs/squashfs/sqfs_dir.c +++ b/fs/squashfs/sqfs_dir.c @@ -7,8 +7,7 @@
#include <errno.h> #include <linux/types.h> -#include <linux/byteorder/little_endian.h> -#include <linux/byteorder/generic.h> +#include <asm/byteorder.h> #include <stdint.h> #include <stdio.h>
#include <stdlib.h>
2.20.1
PING?
PING?
On Wed, Apr 06, 2022 at 11:31:53PM +0200, Pali Rohár wrote:
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Miquel Raynal miquel.raynal@bootlin.com
Applied to u-boot/master, thanks!
participants (3)
-
Miquel Raynal -
Pali Rohár -
Tom Rini