diff mbox series

[v3,15/49] include/exec: Move gdb_stat and gdb_timeval to gdbstub.h

Message ID 20220521000400.454525-16-richard.henderson@linaro.org
State Superseded
Headers show
Series semihosting cleanup | expand

Commit Message

Richard Henderson May 21, 2022, 12:03 a.m. UTC
We have two copies of these structures, and require them
in semihosting/ going forward.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/gdbstub.h    | 25 +++++++++++++++++++++++++
 target/m68k/m68k-semi.c   | 30 +++---------------------------
 target/nios2/nios2-semi.c | 30 +++---------------------------
 3 files changed, 31 insertions(+), 54 deletions(-)

Comments

Peter Maydell May 23, 2022, 12:42 p.m. UTC | #1
On Sat, 21 May 2022 at 01:04, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have two copies of these structures, and require them
> in semihosting/ going forward.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/gdbstub.h    | 25 +++++++++++++++++++++++++
>  target/m68k/m68k-semi.c   | 30 +++---------------------------
>  target/nios2/nios2-semi.c | 30 +++---------------------------
>  3 files changed, 31 insertions(+), 54 deletions(-)
>
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index 2aaba9c723..33a262a5a3 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -20,6 +20,31 @@
>  #define GDB_O_TRUNC   0x400
>  #define GDB_O_EXCL    0x800
>
> +/* For gdb file i/o stat/fstat. */
> +typedef uint32_t gdb_mode_t;
> +typedef uint32_t gdb_time_t;
> +
> +struct gdb_stat {
> +  uint32_t    gdb_st_dev;     /* device */
> +  uint32_t    gdb_st_ino;     /* inode */
> +  gdb_mode_t  gdb_st_mode;    /* protection */
> +  uint32_t    gdb_st_nlink;   /* number of hard links */
> +  uint32_t    gdb_st_uid;     /* user ID of owner */
> +  uint32_t    gdb_st_gid;     /* group ID of owner */
> +  uint32_t    gdb_st_rdev;    /* device type (if inode device) */
> +  uint64_t    gdb_st_size;    /* total size, in bytes */
> +  uint64_t    gdb_st_blksize; /* blocksize for filesystem I/O */
> +  uint64_t    gdb_st_blocks;  /* number of blocks allocated */
> +  gdb_time_t  gdb_st_atime;   /* time of last access */
> +  gdb_time_t  gdb_st_mtime;   /* time of last modification */
> +  gdb_time_t  gdb_st_ctime;   /* time of last change */
> +} QEMU_PACKED;
> +
> +struct gdb_timeval {
> +  gdb_time_t tv_sec;  /* second */
> +  uint64_t tv_usec;   /* microsecond */
> +} QEMU_PACKED;

As an aside,
https://sourceware.org/gdb/onlinedocs/gdb/struct-timeval.html#struct-timeval
says "this structure is of size 8 bytes", but looking at the gdb sources
our definition here is correct and it's 12 bytes (not 8 as the text says
and not 16 as you might expect from the C struct in the docs...)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Alex Bennée June 7, 2022, 1:59 p.m. UTC | #2
Richard Henderson <richard.henderson@linaro.org> writes:

> We have two copies of these structures, and require them
> in semihosting/ going forward.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/gdbstub.h    | 25 +++++++++++++++++++++++++
>  target/m68k/m68k-semi.c   | 30 +++---------------------------
>  target/nios2/nios2-semi.c | 30 +++---------------------------
>  3 files changed, 31 insertions(+), 54 deletions(-)
>
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index 2aaba9c723..33a262a5a3 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -20,6 +20,31 @@
>  #define GDB_O_TRUNC   0x400
>  #define GDB_O_EXCL    0x800
>  
> +/* For gdb file i/o stat/fstat. */
> +typedef uint32_t gdb_mode_t;
> +typedef uint32_t gdb_time_t;
> +
> +struct gdb_stat {
> +  uint32_t    gdb_st_dev;     /* device */
> +  uint32_t    gdb_st_ino;     /* inode */
> +  gdb_mode_t  gdb_st_mode;    /* protection */
> +  uint32_t    gdb_st_nlink;   /* number of hard links */
> +  uint32_t    gdb_st_uid;     /* user ID of owner */
> +  uint32_t    gdb_st_gid;     /* group ID of owner */
> +  uint32_t    gdb_st_rdev;    /* device type (if inode device) */
> +  uint64_t    gdb_st_size;    /* total size, in bytes */
> +  uint64_t    gdb_st_blksize; /* blocksize for filesystem I/O */
> +  uint64_t    gdb_st_blocks;  /* number of blocks allocated */
> +  gdb_time_t  gdb_st_atime;   /* time of last access */
> +  gdb_time_t  gdb_st_mtime;   /* time of last modification */
> +  gdb_time_t  gdb_st_ctime;   /* time of last change */
> +} QEMU_PACKED;
> +
> +struct gdb_timeval {
> +  gdb_time_t tv_sec;  /* second */
> +  uint64_t tv_usec;   /* microsecond */
> +} QEMU_PACKED;
> +
>  #ifdef NEED_CPU_H
>  #include "cpu.h"
>  
> diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
> index 475a6b13b7..da0186f3ef 100644
> --- a/target/m68k/m68k-semi.c
> +++ b/target/m68k/m68k-semi.c
> @@ -45,30 +45,6 @@
>  #define HOSTED_ISATTY 12
>  #define HOSTED_SYSTEM 13
>  
> -typedef uint32_t gdb_mode_t;
> -typedef uint32_t gdb_time_t;
> -
> -struct m68k_gdb_stat {
> -  uint32_t    gdb_st_dev;     /* device */
> -  uint32_t    gdb_st_ino;     /* inode */
> -  gdb_mode_t  gdb_st_mode;    /* protection */
> -  uint32_t    gdb_st_nlink;   /* number of hard links */
> -  uint32_t    gdb_st_uid;     /* user ID of owner */
> -  uint32_t    gdb_st_gid;     /* group ID of owner */
> -  uint32_t    gdb_st_rdev;    /* device type (if inode device) */
> -  uint64_t    gdb_st_size;    /* total size, in bytes */
> -  uint64_t    gdb_st_blksize; /* blocksize for filesystem I/O */
> -  uint64_t    gdb_st_blocks;  /* number of blocks allocated */
> -  gdb_time_t  gdb_st_atime;   /* time of last access */
> -  gdb_time_t  gdb_st_mtime;   /* time of last modification */
> -  gdb_time_t  gdb_st_ctime;   /* time of last change */
> -} QEMU_PACKED;
> -
> -struct gdb_timeval {
> -  gdb_time_t tv_sec;  /* second */
> -  uint64_t tv_usec;   /* microsecond */
> -} QEMU_PACKED;
> -
>  static int translate_openflags(int flags)
>  {
>      int hf;
> @@ -90,9 +66,9 @@ static int translate_openflags(int flags)
>  
>  static void translate_stat(CPUM68KState *env, target_ulong addr, struct stat *s)
>  {
> -    struct m68k_gdb_stat *p;
> +    struct gdb_stat *p;
>  
> -    if (!(p = lock_user(VERIFY_WRITE, addr, sizeof(struct m68k_gdb_stat), 0)))
> +    if (!(p = lock_user(VERIFY_WRITE, addr, sizeof(struct gdb_stat),
> 0)))

checkpatch hard fails on the assignment in an if condition so it's
probably worth cleaning that up while you go.

Otherwise:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
diff mbox series

Patch

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 2aaba9c723..33a262a5a3 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -20,6 +20,31 @@ 
 #define GDB_O_TRUNC   0x400
 #define GDB_O_EXCL    0x800
 
+/* For gdb file i/o stat/fstat. */
+typedef uint32_t gdb_mode_t;
+typedef uint32_t gdb_time_t;
+
+struct gdb_stat {
+  uint32_t    gdb_st_dev;     /* device */
+  uint32_t    gdb_st_ino;     /* inode */
+  gdb_mode_t  gdb_st_mode;    /* protection */
+  uint32_t    gdb_st_nlink;   /* number of hard links */
+  uint32_t    gdb_st_uid;     /* user ID of owner */
+  uint32_t    gdb_st_gid;     /* group ID of owner */
+  uint32_t    gdb_st_rdev;    /* device type (if inode device) */
+  uint64_t    gdb_st_size;    /* total size, in bytes */
+  uint64_t    gdb_st_blksize; /* blocksize for filesystem I/O */
+  uint64_t    gdb_st_blocks;  /* number of blocks allocated */
+  gdb_time_t  gdb_st_atime;   /* time of last access */
+  gdb_time_t  gdb_st_mtime;   /* time of last modification */
+  gdb_time_t  gdb_st_ctime;   /* time of last change */
+} QEMU_PACKED;
+
+struct gdb_timeval {
+  gdb_time_t tv_sec;  /* second */
+  uint64_t tv_usec;   /* microsecond */
+} QEMU_PACKED;
+
 #ifdef NEED_CPU_H
 #include "cpu.h"
 
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index 475a6b13b7..da0186f3ef 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -45,30 +45,6 @@ 
 #define HOSTED_ISATTY 12
 #define HOSTED_SYSTEM 13
 
-typedef uint32_t gdb_mode_t;
-typedef uint32_t gdb_time_t;
-
-struct m68k_gdb_stat {
-  uint32_t    gdb_st_dev;     /* device */
-  uint32_t    gdb_st_ino;     /* inode */
-  gdb_mode_t  gdb_st_mode;    /* protection */
-  uint32_t    gdb_st_nlink;   /* number of hard links */
-  uint32_t    gdb_st_uid;     /* user ID of owner */
-  uint32_t    gdb_st_gid;     /* group ID of owner */
-  uint32_t    gdb_st_rdev;    /* device type (if inode device) */
-  uint64_t    gdb_st_size;    /* total size, in bytes */
-  uint64_t    gdb_st_blksize; /* blocksize for filesystem I/O */
-  uint64_t    gdb_st_blocks;  /* number of blocks allocated */
-  gdb_time_t  gdb_st_atime;   /* time of last access */
-  gdb_time_t  gdb_st_mtime;   /* time of last modification */
-  gdb_time_t  gdb_st_ctime;   /* time of last change */
-} QEMU_PACKED;
-
-struct gdb_timeval {
-  gdb_time_t tv_sec;  /* second */
-  uint64_t tv_usec;   /* microsecond */
-} QEMU_PACKED;
-
 static int translate_openflags(int flags)
 {
     int hf;
@@ -90,9 +66,9 @@  static int translate_openflags(int flags)
 
 static void translate_stat(CPUM68KState *env, target_ulong addr, struct stat *s)
 {
-    struct m68k_gdb_stat *p;
+    struct gdb_stat *p;
 
-    if (!(p = lock_user(VERIFY_WRITE, addr, sizeof(struct m68k_gdb_stat), 0)))
+    if (!(p = lock_user(VERIFY_WRITE, addr, sizeof(struct gdb_stat), 0)))
         /* FIXME - should this return an error code? */
         return;
     p->gdb_st_dev = cpu_to_be32(s->st_dev);
@@ -114,7 +90,7 @@  static void translate_stat(CPUM68KState *env, target_ulong addr, struct stat *s)
     p->gdb_st_atime = cpu_to_be32(s->st_atime);
     p->gdb_st_mtime = cpu_to_be32(s->st_mtime);
     p->gdb_st_ctime = cpu_to_be32(s->st_ctime);
-    unlock_user(p, addr, sizeof(struct m68k_gdb_stat));
+    unlock_user(p, addr, sizeof(struct gdb_stat));
 }
 
 static void m68k_semi_return_u32(CPUM68KState *env, uint32_t ret, uint32_t err)
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index 0eec1f9a1c..3e504a6c5f 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -47,30 +47,6 @@ 
 #define HOSTED_ISATTY 12
 #define HOSTED_SYSTEM 13
 
-typedef uint32_t gdb_mode_t;
-typedef uint32_t gdb_time_t;
-
-struct nios2_gdb_stat {
-  uint32_t    gdb_st_dev;     /* device */
-  uint32_t    gdb_st_ino;     /* inode */
-  gdb_mode_t  gdb_st_mode;    /* protection */
-  uint32_t    gdb_st_nlink;   /* number of hard links */
-  uint32_t    gdb_st_uid;     /* user ID of owner */
-  uint32_t    gdb_st_gid;     /* group ID of owner */
-  uint32_t    gdb_st_rdev;    /* device type (if inode device) */
-  uint64_t    gdb_st_size;    /* total size, in bytes */
-  uint64_t    gdb_st_blksize; /* blocksize for filesystem I/O */
-  uint64_t    gdb_st_blocks;  /* number of blocks allocated */
-  gdb_time_t  gdb_st_atime;   /* time of last access */
-  gdb_time_t  gdb_st_mtime;   /* time of last modification */
-  gdb_time_t  gdb_st_ctime;   /* time of last change */
-} QEMU_PACKED;
-
-struct gdb_timeval {
-  gdb_time_t tv_sec;  /* second */
-  uint64_t tv_usec;   /* microsecond */
-} QEMU_PACKED;
-
 static int translate_openflags(int flags)
 {
     int hf;
@@ -102,9 +78,9 @@  static int translate_openflags(int flags)
 static bool translate_stat(CPUNios2State *env, target_ulong addr,
                            struct stat *s)
 {
-    struct nios2_gdb_stat *p;
+    struct gdb_stat *p;
 
-    p = lock_user(VERIFY_WRITE, addr, sizeof(struct nios2_gdb_stat), 0);
+    p = lock_user(VERIFY_WRITE, addr, sizeof(struct gdb_stat), 0);
 
     if (!p) {
         return false;
@@ -128,7 +104,7 @@  static bool translate_stat(CPUNios2State *env, target_ulong addr,
     p->gdb_st_atime = cpu_to_be32(s->st_atime);
     p->gdb_st_mtime = cpu_to_be32(s->st_mtime);
     p->gdb_st_ctime = cpu_to_be32(s->st_ctime);
-    unlock_user(p, addr, sizeof(struct nios2_gdb_stat));
+    unlock_user(p, addr, sizeof(struct gdb_stat));
     return true;
 }