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

[Alex Bennée]

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>

-- 
Alex Bennée