| Message ID | 20220521000400.454525-16-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | semihosting cleanup | expand |
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
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 --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; }
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(-)