[Qemu-devel] [PATCH v2] linux-user: Fix stat64 syscall for SPARC64

[Qemu-devel] [PATCH v2] linux-user: Fix stat64 syscall for SPARC64

[Stefan Weil]

Some targets use a stat64 structure for the stat64 syscall while others
use a stat structure. SPARC64 used the wrong kind.

Instead of extending the conditional compilation in syscall.c, now a
macro TARGET_HAS_STRUCT_STAT64 is defined whenever a target has a
target_stat64.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

v2:
This is an update of this previous patch:

linux-user: Fix wrong use of stat instead of stat64 for sparc64
(http://patchwork.ozlabs.org/patch/273269/)

Peter Maydell suggested a new macro TARGET_NO_STRUCT_STAT64
to mark sections without a target_stat64 definition.

I implemented a slighty different solution here and marked
sections with that definition, so it is easier to review.

For PPC64 without 32 bit API, I had to add a preprocessor
check to avoid usage of the wrong struct.

Regards,
Stefan

 linux-user/syscall.c      |    6 +++---
 linux-user/syscall_defs.h |   14 ++++++++++++++
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 4a14a43..eaaf00d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -4870,10 +4870,10 @@ static inline abi_long host_to_target_stat64(void *cpu_env,
     } else
 #endif
     {
-#if TARGET_ABI_BITS == 64 && !defined(TARGET_ALPHA)
-        struct target_stat *target_st;
-#else
+#if defined(TARGET_HAS_STRUCT_STAT64)
         struct target_stat64 *target_st;
+#else
+        struct target_stat *target_st;
 #endif
 
         if (!lock_user_struct(VERIFY_WRITE, target_st, target_addr, 0))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 5f53a28..fe540f6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1178,6 +1178,7 @@ struct target_stat {
 /* This matches struct stat64 in glibc2.1, hence the absolutely
  * insane amounts of padding around dev_t's.
  */
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
 	unsigned short	st_dev;
 	unsigned char	__pad0[10];
@@ -1213,6 +1214,7 @@ struct target_stat64 {
 } QEMU_PACKED;
 
 #ifdef TARGET_ARM
+#define TARGET_HAS_STRUCT_STAT64
 struct target_eabi_stat64 {
         unsigned long long st_dev;
         unsigned int    __pad1;
@@ -1262,6 +1264,7 @@ struct target_stat {
 	abi_ulong	__unused4[2];
 };
 
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
 	unsigned char	__pad0[6];
 	unsigned short	st_dev;
@@ -1317,6 +1320,7 @@ struct target_stat {
 	abi_ulong	__unused4[2];
 };
 
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
 	unsigned char	__pad0[6];
 	unsigned short	st_dev;
@@ -1384,6 +1388,8 @@ struct target_stat {
 #endif
 };
 
+#if !defined(TARGET_PPC64) || defined(TARGET_ABI32)
+#define TARGET_HAS_STRUCT_STAT64
 struct QEMU_PACKED target_stat64 {
 	unsigned long long st_dev;
         unsigned long long st_ino;
@@ -1406,6 +1412,7 @@ struct QEMU_PACKED target_stat64 {
         unsigned int   __unused4;
         unsigned int   __unused5;
 };
+#endif
 
 #elif defined(TARGET_MICROBLAZE)
 
@@ -1431,6 +1438,7 @@ struct target_stat {
 };
 
 /* FIXME: Microblaze no-mmu user-space has a difference stat64 layout...  */
+#define TARGET_HAS_STRUCT_STAT64
 struct QEMU_PACKED target_stat64 {
 	uint64_t st_dev;
 #define TARGET_STAT64_HAS_BROKEN_ST_INO 1
@@ -1486,6 +1494,7 @@ struct target_stat {
 /* This matches struct stat64 in glibc2.1, hence the absolutely
  * insane amounts of padding around dev_t's.
  */
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
 	unsigned long long	st_dev;
 	unsigned char	__pad1[2];
@@ -1594,6 +1603,7 @@ struct target_stat {
  * struct stat of the 64-bit kernel.
  */
 
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
 	unsigned int	st_dev;
 	unsigned int	st_pad0[3];	/* Reserved for st_dev expansion  */
@@ -1665,6 +1675,7 @@ struct target_stat {
  * struct stat of the 64-bit kernel.
  */
 
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
 	abi_ulong	st_dev;
 	abi_ulong	st_pad0[3];	/* Reserved for st_dev expansion  */
@@ -1721,6 +1732,7 @@ struct target_stat {
        unsigned int    st_gen;
 };
 
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
        abi_ulong    st_dev;
        abi_ulong    st_ino;
@@ -1770,6 +1782,7 @@ struct target_stat {
 /* This matches struct stat64 in glibc2.1, hence the absolutely
  * insane amounts of padding around dev_t's.
  */
+#define TARGET_HAS_STRUCT_STAT64
 struct QEMU_PACKED target_stat64 {
 	unsigned long long	st_dev;
 	unsigned char	__pad0[4];
@@ -1897,6 +1910,7 @@ struct target_stat {
     unsigned int __unused5;
 };
 
+#define TARGET_HAS_STRUCT_STAT64
 struct target_stat64 {
     uint64_t st_dev;
     uint64_t st_ino;
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH v2] linux-user: Fix stat64 syscall for SPARC64

[Erik de Castro Lopo]

Stefan Weil wrote:

> Some targets use a stat64 structure for the stat64 syscall while others
> use a stat structure. SPARC64 used the wrong kind.
> 
> Instead of extending the conditional compilation in syscall.c, now a
> macro TARGET_HAS_STRUCT_STAT64 is defined whenever a target has a
> target_stat64.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Reviewed-by: Erik de Castro Lopo <erikd@mega-nerd.com>


-- 
----------------------------------------------------------------------
Erik de Castro Lopo
http://www.mega-nerd.com/

Re: [Qemu-devel] [PATCH v2] linux-user: Fix stat64 syscall for SPARC64

[Stefan Weil]

Am 31.10.2013 05:33, schrieb Erik de Castro Lopo:
> Stefan Weil wrote:
>
>> Some targets use a stat64 structure for the stat64 syscall while others
>> use a stat structure. SPARC64 used the wrong kind.
>>
>> Instead of extending the conditional compilation in syscall.c, now a
>> macro TARGET_HAS_STRUCT_STAT64 is defined whenever a target has a
>> target_stat64.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> Reviewed-by: Erik de Castro Lopo <erikd@mega-nerd.com>

Ping?

Peter, maybe you want to review this patch? I slightly modified your
suggestion.

Riku, could you please include the patch in your queue (allow Peter to
review it before)?

Thanks,
Stefan