* [PATCH v3] Allow using special files with File I/O functions [not found] <20180628192635.44056-1-julio@farjump.io> @ 2018-06-28 19:27 ` Julio Guerra [not found] ` <b725e8cb-4a89-b204-1492-d8af5c76b89f@farjump.io> 2018-06-29 13:42 ` Pedro Alves 0 siblings, 2 replies; 9+ messages in thread From: Julio Guerra @ 2018-06-28 19:27 UTC (permalink / raw) To: gdb-patches; +Cc: Pedro Alves, Julio Guerra - Remove the restriction to regular files only and add support for special file types in the File IO stat structure. - Define a few more macro definitions of file types such as FIFOs, etc. The major goal is being able to write advanced embedded testing functions, like: - using a FIFO between the embedded program and the host, instead of being restricted only to the GDB console. - mocking features based on host's by opening some /dev special files. 2018-06-28 Julio Guerra <julio@farjump.io> * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat): Allow using File I/O functions open(), stat() and fstat() on special files. * ../include/gdb/fileio.h: Add macro definitions for special files, both for fst_dev and fst_mode fields of struct fst_stat. * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new special file types in fst_mode's definition. (host_to_fileio_stat): Define fst_dev using the new macro definitions and according to the file's type. Signed-off-by: Julio Guerra <julio@farjump.io> --- gdb/ChangeLog | 12 ++++++++ gdb/common/fileio.c | 66 +++++++++++++++++++++++++++++++++++--------- gdb/remote-fileio.c | 28 ++----------------- include/gdb/fileio.h | 19 +++++++++++-- 4 files changed, 83 insertions(+), 42 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 18c1915675..19cf5e38a6 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,15 @@ +2018-06-28 Julio Guerra <julio@farjump.io> + + * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat): + Allow using File I/O functions open(), stat() and fstat() on special + files. + * ../include/gdb/fileio.h: Add macro definitions for special files, + both for fst_dev and fst_mode fields of struct fst_stat. + * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new + special file types in fst_mode's definition. + (host_to_fileio_stat): Define fst_dev using the new macro definitions + and according to the file's type. + 2018-06-27 Simon Marchi <simon.marchi@ericsson.com> * gdb-gdb.py.in (StructMainTypePrettyPrinter) <to_string>: Don't diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c index 912a7ede3c..9ee78e227c 100644 --- a/gdb/common/fileio.c +++ b/gdb/common/fileio.c @@ -119,12 +119,31 @@ fileio_to_host_mode (int fileio_mode, mode_t *mode_p) if (fileio_mode & ~FILEIO_S_SUPPORTED) return -1; - if (fileio_mode & FILEIO_S_IFREG) - mode |= S_IFREG; - if (fileio_mode & FILEIO_S_IFDIR) - mode |= S_IFDIR; - if (fileio_mode & FILEIO_S_IFCHR) - mode |= S_IFCHR; + switch (fileio_mode & FILEIO_S_IFMT) + { + case FILEIO_S_IFSOCK: + *mode_p |= S_IFSOCK; + break; + case FILEIO_S_IFLNK: + mode |= S_IFLNK; + break; + case FILEIO_S_IFREG: + mode |= S_IFREG; + break; + case FILEIO_S_IFBLK: + mode |= S_IFBLK; + break; + case FILEIO_S_IFDIR: + mode |= S_IFDIR; + break; + case FILEIO_S_IFCHR: + mode |= S_IFCHR; + break; + case FILEIO_S_IFIFO: + mode |= S_IFIFO; + break; + } + if (fileio_mode & FILEIO_S_IRUSR) mode |= S_IRUSR; if (fileio_mode & FILEIO_S_IWUSR) @@ -165,12 +184,31 @@ fileio_mode_pack (mode_t mode) { mode_t tmode = 0; - if (S_ISREG (mode)) - tmode |= FILEIO_S_IFREG; - if (S_ISDIR (mode)) - tmode |= FILEIO_S_IFDIR; - if (S_ISCHR (mode)) - tmode |= FILEIO_S_IFCHR; + switch (mode & S_IFMT) + { + case S_IFSOCK: + tmode |= FILEIO_S_IFSOCK; + break; + case S_IFLNK: + tmode |= FILEIO_S_IFLNK; + break; + case S_IFREG: + tmode |= FILEIO_S_IFREG; + break; + case S_IFBLK: + tmode |= FILEIO_S_IFBLK; + break; + case S_IFDIR: + tmode |= FILEIO_S_IFDIR; + break; + case S_IFCHR: + tmode |= FILEIO_S_IFCHR; + break; + case S_IFIFO: + tmode |= FILEIO_S_IFIFO; + break; + } + if (mode & S_IRUSR) tmode |= FILEIO_S_IRUSR; if (mode & S_IWUSR) @@ -224,8 +262,10 @@ void host_to_fileio_stat (struct stat *st, struct fio_stat *fst) { LONGEST blksize; + long fst_dev; - host_to_fileio_uint ((long) st->st_dev, fst->fst_dev); + fst_dev = S_ISREG(st->st_mode) ? FILEIO_STDEV_FILE : FILEIO_STDEV_SPECIAL; + host_to_fileio_uint (fst_dev, fst->fst_dev); host_to_fileio_uint ((long) st->st_ino, fst->fst_ino); host_to_fileio_mode (st->st_mode, fst->fst_mode); host_to_fileio_uint ((long) st->st_nlink, fst->fst_nlink); diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c index 313da642ea..837081269a 100644 --- a/gdb/remote-fileio.c +++ b/gdb/remote-fileio.c @@ -407,24 +407,6 @@ remote_fileio_func_open (remote_target *remote, char *buf) return; } - /* Check if pathname exists and is not a regular file or directory. If so, - return an appropriate error code. Same for trying to open directories - for writing. */ - if (!stat (pathname, &st)) - { - if (!S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode)) - { - remote_fileio_reply (remote, -1, FILEIO_ENODEV); - return; - } - if (S_ISDIR (st.st_mode) - && ((flags & O_WRONLY) == O_WRONLY || (flags & O_RDWR) == O_RDWR)) - { - remote_fileio_reply (remote, -1, FILEIO_EISDIR); - return; - } - } - fd = gdb_open_cloexec (pathname, flags, mode); if (fd < 0) { @@ -885,16 +867,9 @@ remote_fileio_func_stat (remote_target *remote, char *buf) remote_fileio_return_errno (remote, -1); return; } - /* Only operate on regular files and directories. */ - if (!ret && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode)) - { - remote_fileio_reply (remote, -1, FILEIO_EACCES); - return; - } if (statptr) { host_to_fileio_stat (&st, &fst); - host_to_fileio_uint (0, fst.fst_dev); errno = target_write_memory (statptr, (gdb_byte *) &fst, sizeof fst); if (errno != 0) @@ -939,7 +914,6 @@ remote_fileio_func_fstat (remote_target *remote, char *buf) if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT) { - host_to_fileio_uint (1, fst.fst_dev); memset (&st, 0, sizeof (st)); st.st_mode = S_IFCHR | (fd == FIO_FD_CONSOLE_IN ? S_IRUSR : S_IWUSR); st.st_nlink = 1; @@ -972,6 +946,8 @@ remote_fileio_func_fstat (remote_target *remote, char *buf) if (ptrval) { host_to_fileio_stat (&st, &fst); + if (fd == FIO_FD_CONSOLE_IN || fd == FIO_FD_CONSOLE_OUT) + host_to_fileio_uint (FILEIO_STDEV_CONSOLE, fst.fst_dev); errno = target_write_memory (ptrval, (gdb_byte *) &fst, sizeof fst); if (errno != 0) diff --git a/include/gdb/fileio.h b/include/gdb/fileio.h index 7bb55f579f..ada84c5f10 100644 --- a/include/gdb/fileio.h +++ b/include/gdb/fileio.h @@ -37,10 +37,24 @@ FILEIO_O_CREAT | FILEIO_O_TRUNC| \ FILEIO_O_EXCL) +/* Device id values of fst_dev field */ +/* Regular file */ +#define FILEIO_STDEV_FILE 0 +/* GDB's console */ +#define FILEIO_STDEV_CONSOLE 1 +/* Not a regular file nor the console. + Bits FILEIO_S_IFMT of fst_mode give the exact file type. */ +#define FILEIO_STDEV_SPECIAL 2 + /* mode_t bits */ +#define FILEIO_S_IFSOCK 0140000 +#define FILEIO_S_IFLNK 0120000 #define FILEIO_S_IFREG 0100000 +#define FILEIO_S_IFBLK 060000 #define FILEIO_S_IFDIR 040000 #define FILEIO_S_IFCHR 020000 +#define FILEIO_S_IFIFO 010000 +#define FILEIO_S_IFMT 0170000 #define FILEIO_S_IRUSR 0400 #define FILEIO_S_IWUSR 0200 #define FILEIO_S_IXUSR 0100 @@ -53,9 +67,8 @@ #define FILEIO_S_IWOTH 02 #define FILEIO_S_IXOTH 01 #define FILEIO_S_IRWXO 07 -#define FILEIO_S_SUPPORTED (FILEIO_S_IFREG|FILEIO_S_IFDIR| \ - FILEIO_S_IRWXU|FILEIO_S_IRWXG| \ - FILEIO_S_IRWXO) +#define FILEIO_S_SUPPORTED (FILEIO_S_IFMT | FILEIO_S_IRWXU| \ + FILEIO_S_IRWXG | FILEIO_S_IRWXO) /* lseek(2) flags */ #define FILEIO_SEEK_SET 0 -- 2.18.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <b725e8cb-4a89-b204-1492-d8af5c76b89f@farjump.io>]
* Re: [PATCH v3] Allow using special files with File I/O functions [not found] ` <b725e8cb-4a89-b204-1492-d8af5c76b89f@farjump.io> @ 2018-06-28 19:31 ` Julio Guerra 0 siblings, 0 replies; 9+ messages in thread From: Julio Guerra @ 2018-06-28 19:31 UTC (permalink / raw) To: gdb-patches; +Cc: Pedro Alves You can find below the logs of the test I performed using our GDB server with a program embedded in a Raspberry Pi: (gdb) set $st = (struct stat*) malloc(sizeof (struct stat)) (gdb) call stat("/dev/stdout", $st) $1 = 0x0 (gdb) p *$st $2 = { st_dev = 0x2, st_ino = 0x4, st_mode = 0x2190, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x5, st_rdev = 0x8801, st_size = 0x0, st_atime = 0x5b3507e0, st_spare1 = 0x0, st_mtime = 0x5b34fb47, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b3507e0, st_spare4 = {0x0, 0x0} } (gdb) p /o $st->st_mode $8 = 020620 (gdb) call stat("myfifo", $st) $3 = 0x0 (gdb) p *$st $4 = { st_dev = 0x2, st_ino = 0x4f7c, st_mode = 0x11a4, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x3e8, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b3504ed, st_spare1 = 0x0, st_mtime = 0x5b3504ed, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b3504ed, st_spare4 = {0x0, 0x0} } (gdb) p /o $st->st_mode $6 = 010644 (gdb) call stat("/run/dbus/system_bus_socket", $st) $9 = 0x0 (gdb) p *$st $10 = { st_dev = 0x2, st_ino = 0x36c7, st_mode = 0xc1b6, st_nlink = 0x1, st_uid = 0x0, st_gid = 0x0, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b33e995, st_spare1 = 0x0, st_mtime = 0x5b33e995, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b33e995, st_spare4 = {0x0, 0x0} } (gdb) p /o $st->st_mode $11 = 0140666 (gdb) call stat("/tmp", $st) $12 = 0x0 (gdb) p *$st $13 = { st_dev = 0x2, st_ino = 0x7a5, st_mode = 0x41ff, st_nlink = 0x1, st_uid = 0x0, st_gid = 0x0, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b350829, st_spare1 = 0x0, st_mtime = 0x5b350829, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5a5a9802, st_spare4 = {0x0, 0x0} } (gdb) p /o $st->st_mode $14 = 040777 (gdb) call stat("Makefile", $st) $15 = 0x0 (gdb) p *$st $16 = { st_dev = 0x0, st_ino = 0xc6b9, st_mode = 0x81a4, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x3e8, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5ae35b23, st_spare1 = 0x0, st_mtime = 0x5b311f46, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b3500d5, st_spare4 = {0x0, 0x0} } (gdb) p /o $st->st_mode $17 = 0100644 (gdb) call stat("i dont exist", $st) $18 = 0xffffffff (gdb) call fstat(0, $st) $1 = 0x0 (gdb) p *$st $3 = { st_dev = 0x1, st_ino = 0x0, st_mode = 0x2100, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x3e8, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b35144e, st_spare1 = 0x0, st_mtime = 0x5b35144e, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b35144e, st_spare4 = {0x0, 0x0} } (gdb) call fstat(1, $st) $4 = 0x0 (gdb) p *$st $5 = { st_dev = 0x1, st_ino = 0x0, st_mode = 0x2080, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x3e8, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b351460, st_spare1 = 0x0, st_mtime = 0x5b351460, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b351460, st_spare4 = {0x0, 0x0} } (gdb) call fstat(2, $st) $6 = 0x0 (gdb) p *$st $7 = { st_dev = 0x1, st_ino = 0x0, st_mode = 0x2080, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x3e8, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b35146b, st_spare1 = 0x0, st_mtime = 0x5b35146b, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b35146b, st_spare4 = {0x0, 0x0} } (gdb) call fstat(3, $st) $8 = 0xffffffff (gdb) set $fd = open("myfifo", 0) (gdb) call fstat($fd, $st) $10 = 0x0 (gdb) p *$st $12 = { st_dev = 0x2, st_ino = 0x4f7c, st_mode = 0x11a4, st_nlink = 0x1, st_uid = 0x3e8, st_gid = 0x3e8, st_rdev = 0x0, st_size = 0x0, st_atime = 0x5b351511, st_spare1 = 0x0, st_mtime = 0x5b351511, st_spare2 = 0x0, st_ctime = 0x0, st_spare3 = 0x0, st_blksize = 0x0, st_blocks = 0x5b3504ed, st_spare4 = {0x0, 0x0} } (gdb) p /o $st->st_mode $13 = 010644 -- Julio Guerra Co-founder & CTO of Farjump Mobile: +33 618 644 164 LinkedIn: https://linkedin.com/in/guerrajulio Slack: farjump.slack.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] Allow using special files with File I/O functions 2018-06-28 19:27 ` [PATCH v3] Allow using special files with File I/O functions Julio Guerra [not found] ` <b725e8cb-4a89-b204-1492-d8af5c76b89f@farjump.io> @ 2018-06-29 13:42 ` Pedro Alves [not found] ` <79758ca1-2541-9ae6-d793-b367d6094468@farjump.io> 1 sibling, 1 reply; 9+ messages in thread From: Pedro Alves @ 2018-06-29 13:42 UTC (permalink / raw) To: Julio Guerra, gdb-patches On 06/28/2018 08:27 PM, Julio Guerra wrote: > - Remove the restriction to regular files only and add support for special file > types in the File IO stat structure. > - Define a few more macro definitions of file types such as FIFOs, etc. > > The major goal is being able to write advanced embedded testing functions, like: > - using a FIFO between the embedded program and the host, instead of being > restricted only to the GDB console. > - mocking features based on host's by opening some /dev special files. > This needs a GDB manual change, and a NEWS entry. > 2018-06-28 Julio Guerra <julio@farjump.io> > > * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat): > Allow using File I/O functions open(), stat() and fstat() on special > files. > * ../include/gdb/fileio.h: Add macro definitions for special files, > both for fst_dev and fst_mode fields of struct fst_stat. > * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new > special file types in fst_mode's definition. > (host_to_fileio_stat): Define fst_dev using the new macro definitions > and according to the file's type. Note that include/ has its own ChangeLog file. > 2018-06-27 Simon Marchi <simon.marchi@ericsson.com> > > * gdb-gdb.py.in (StructMainTypePrettyPrinter) <to_string>: Don't > diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c > index 912a7ede3c..9ee78e227c 100644 > --- a/gdb/common/fileio.c > +++ b/gdb/common/fileio.c > @@ -119,12 +119,31 @@ fileio_to_host_mode (int fileio_mode, mode_t *mode_p) > if (fileio_mode & ~FILEIO_S_SUPPORTED) > return -1; > > - if (fileio_mode & FILEIO_S_IFREG) > - mode |= S_IFREG; > - if (fileio_mode & FILEIO_S_IFDIR) > - mode |= S_IFDIR; > - if (fileio_mode & FILEIO_S_IFCHR) > - mode |= S_IFCHR; > + switch (fileio_mode & FILEIO_S_IFMT) > + { > + case FILEIO_S_IFSOCK: > + *mode_p |= S_IFSOCK; > + break; > + case FILEIO_S_IFLNK: > + mode |= S_IFLNK; > + break; > + case FILEIO_S_IFREG: > + mode |= S_IFREG; > + break; > + case FILEIO_S_IFBLK: > + mode |= S_IFBLK; > + break; > + case FILEIO_S_IFDIR: > + mode |= S_IFDIR; > + break; > + case FILEIO_S_IFCHR: > + mode |= S_IFCHR; > + break; > + case FILEIO_S_IFIFO: > + mode |= S_IFIFO; > + break; > + } > + > if (fileio_mode & FILEIO_S_IRUSR) > mode |= S_IRUSR; > if (fileio_mode & FILEIO_S_IWUSR) > @@ -165,12 +184,31 @@ fileio_mode_pack (mode_t mode) > { > mode_t tmode = 0; > > - if (S_ISREG (mode)) > - tmode |= FILEIO_S_IFREG; > - if (S_ISDIR (mode)) > - tmode |= FILEIO_S_IFDIR; > - if (S_ISCHR (mode)) > - tmode |= FILEIO_S_IFCHR; > + switch (mode & S_IFMT) > + { > + case S_IFSOCK: > + tmode |= FILEIO_S_IFSOCK; > + break; > + case S_IFLNK: > + tmode |= FILEIO_S_IFLNK; > + break; > + case S_IFREG: > + tmode |= FILEIO_S_IFREG; > + break; > + case S_IFBLK: > + tmode |= FILEIO_S_IFBLK; > + break; > + case S_IFDIR: > + tmode |= FILEIO_S_IFDIR; > + break; > + case S_IFCHR: > + tmode |= FILEIO_S_IFCHR; > + break; > + case S_IFIFO: > + tmode |= FILEIO_S_IFIFO; > + break; > + } I'm not sure whether all these S_FOO macros exist on all hosts. Looking at: https://www.gnu.org/software/gnulib/manual/html_node/sys_002fstat_002eh.html gnulib's sys/stat.h (gdb/gnulib/import/sys_stat.in.h) adds some replacements, but then I'm not sure whether checking S_IFxxx etc. directly instead of using the S_ISxxx function-style macros is the right thing to do portability-wise. It may be S_ISxxx was being used for a reason? > + > if (mode & S_IRUSR) > tmode |= FILEIO_S_IRUSR; > if (mode & S_IWUSR) > @@ -224,8 +262,10 @@ void > host_to_fileio_stat (struct stat *st, struct fio_stat *fst) > { > LONGEST blksize; > + long fst_dev; We don't align variable names like that. > > - host_to_fileio_uint ((long) st->st_dev, fst->fst_dev); > + fst_dev = S_ISREG(st->st_mode) ? FILEIO_STDEV_FILE : FILEIO_STDEV_SPECIAL; Missing space in "S_ISREG (". > + host_to_fileio_uint (fst_dev, fst->fst_dev); > host_to_fileio_uint ((long) st->st_ino, fst->fst_ino); > host_to_fileio_mode (st->st_mode, fst->fst_mode); > host_to_fileio_uint ((long) st->st_nlink, fst->fst_nlink); > diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c > index 313da642ea..837081269a 100644 > --- a/gdb/remote-fileio.c > +++ b/gdb/remote-fileio.c > @@ -407,24 +407,6 @@ remote_fileio_func_open (remote_target *remote, char *buf) > return; > } > > - /* Check if pathname exists and is not a regular file or directory. If so, > - return an appropriate error code. Same for trying to open directories > - for writing. */ Did you intend to remove the "Same for trying to open directories for writing." part? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <79758ca1-2541-9ae6-d793-b367d6094468@farjump.io>]
* Re: [PATCH v3] Allow using special files with File I/O functions [not found] ` <79758ca1-2541-9ae6-d793-b367d6094468@farjump.io> @ 2018-06-29 14:01 ` Julio Guerra 2018-06-29 14:28 ` Pedro Alves 0 siblings, 1 reply; 9+ messages in thread From: Julio Guerra @ 2018-06-29 14:01 UTC (permalink / raw) To: Pedro Alves, gdb-patches Le 29/06/2018 à 15:42, Pedro Alves a écrit : On 06/28/2018 08:27 PM, Julio Guerra wrote: - Remove the restriction to regular files only and add support for special file types in the File IO stat structure. - Define a few more macro definitions of file types such as FIFOs, etc. The major goal is being able to write advanced embedded testing functions, like: - using a FIFO between the embedded program and the host, instead of being restricted only to the GDB console. - mocking features based on host's by opening some /dev special files. This needs a GDB manual change, and a NEWS entry. Ok. 2018-06-28 Julio Guerra <julio@farjump.io> * remote-fileio.c (remote_fileio_func_open, remote_fileio_func_stat): Allow using File I/O functions open(), stat() and fstat() on special files. * ../include/gdb/fileio.h: Add macro definitions for special files, both for fst_dev and fst_mode fields of struct fst_stat. * common/fileio.c (fileio_to_host_mode, fileio_mode_pack): Add new special file types in fst_mode's definition. (host_to_fileio_stat): Define fst_dev using the new macro definitions and according to the file's type. Note that include/ has its own ChangeLog file. Ok. 2018-06-27 Simon Marchi <simon.marchi@ericsson.com> * gdb-gdb.py.in (StructMainTypePrettyPrinter) <to_string>: Don't diff --git a/gdb/common/fileio.c b/gdb/common/fileio.c index 912a7ede3c..9ee78e227c 100644 --- a/gdb/common/fileio.c +++ b/gdb/common/fileio.c @@ -119,12 +119,31 @@ fileio_to_host_mode (int fileio_mode, mode_t *mode_p) if (fileio_mode & ~FILEIO_S_SUPPORTED) return -1; - if (fileio_mode & FILEIO_S_IFREG) - mode |= S_IFREG; - if (fileio_mode & FILEIO_S_IFDIR) - mode |= S_IFDIR; - if (fileio_mode & FILEIO_S_IFCHR) - mode |= S_IFCHR; + switch (fileio_mode & FILEIO_S_IFMT) + { + case FILEIO_S_IFSOCK: + *mode_p |= S_IFSOCK; + break; + case FILEIO_S_IFLNK: + mode |= S_IFLNK; + break; + case FILEIO_S_IFREG: + mode |= S_IFREG; + break; + case FILEIO_S_IFBLK: + mode |= S_IFBLK; + break; + case FILEIO_S_IFDIR: + mode |= S_IFDIR; + break; + case FILEIO_S_IFCHR: + mode |= S_IFCHR; + break; + case FILEIO_S_IFIFO: + mode |= S_IFIFO; + break; + } + if (fileio_mode & FILEIO_S_IRUSR) mode |= S_IRUSR; if (fileio_mode & FILEIO_S_IWUSR) @@ -165,12 +184,31 @@ fileio_mode_pack (mode_t mode) { mode_t tmode = 0; - if (S_ISREG (mode)) - tmode |= FILEIO_S_IFREG; - if (S_ISDIR (mode)) - tmode |= FILEIO_S_IFDIR; - if (S_ISCHR (mode)) - tmode |= FILEIO_S_IFCHR; + switch (mode & S_IFMT) + { + case S_IFSOCK: + tmode |= FILEIO_S_IFSOCK; + break; + case S_IFLNK: + tmode |= FILEIO_S_IFLNK; + break; + case S_IFREG: + tmode |= FILEIO_S_IFREG; + break; + case S_IFBLK: + tmode |= FILEIO_S_IFBLK; + break; + case S_IFDIR: + tmode |= FILEIO_S_IFDIR; + break; + case S_IFCHR: + tmode |= FILEIO_S_IFCHR; + break; + case S_IFIFO: + tmode |= FILEIO_S_IFIFO; + break; + } I'm not sure whether all these S_FOO macros exist on all hosts. Looking at: https://www.gnu.org/software/gnulib/manual/html_node/sys_002fstat_002eh.html gnulib's sys/stat.h (gdb/gnulib/import/sys_stat.in.h) adds some replacements, but then I'm not sure whether checking S_IFxxx etc. directly instead of using the S_ISxxx function-style macros is the right thing to do portability-wise. It may be S_ISxxx was being used for a reason? I assumed a system >= POSIX.1-2001 here. man 7 inode says: The S_IF* constants are present in POSIX.1-2001 and later. […] The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but both are present in POSIX.1-2001 + if (mode & S_IRUSR) tmode |= FILEIO_S_IRUSR; if (mode & S_IWUSR) @@ -224,8 +262,10 @@ void host_to_fileio_stat (struct stat *st, struct fio_stat *fst) { LONGEST blksize; + long fst_dev; We don't align variable names like that. Ok. - host_to_fileio_uint ((long) st->st_dev, fst->fst_dev); + fst_dev = S_ISREG(st->st_mode) ? FILEIO_STDEV_FILE : FILEIO_STDEV_SPECIAL; Missing space in "S_ISREG (". Ok. + host_to_fileio_uint (fst_dev, fst->fst_dev); host_to_fileio_uint ((long) st->st_ino, fst->fst_ino); host_to_fileio_mode (st->st_mode, fst->fst_mode); host_to_fileio_uint ((long) st->st_nlink, fst->fst_nlink); diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c index 313da642ea..837081269a 100644 --- a/gdb/remote-fileio.c +++ b/gdb/remote-fileio.c @@ -407,24 +407,6 @@ remote_fileio_func_open (remote_target *remote, char *buf) return; } - /* Check if pathname exists and is not a regular file or directory. If so, - return an appropriate error code. Same for trying to open directories - for writing. */ Did you intend to remove the "Same for trying to open directories for writing." part? Yes, because of man 2 open: EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR, or includes O_CREAT without O_DIRECTORY. I wait for your comments until I resubmit a v4. Thank you, -- Julio Guerra ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] Allow using special files with File I/O functions 2018-06-29 14:01 ` Julio Guerra @ 2018-06-29 14:28 ` Pedro Alves [not found] ` <6ea235bf-bc87-256a-e745-e54f5e97bf5c@farjump.io> 0 siblings, 1 reply; 9+ messages in thread From: Pedro Alves @ 2018-06-29 14:28 UTC (permalink / raw) To: Julio Guerra, gdb-patches Hi, Your message came our hard to read: https://sourceware.org/ml/gdb-patches/2018-06/msg00715.html Please make sure your client is set up to quote replies. On 06/29/2018 03:01 PM, Julio Guerra wrote: > I assumed a system >= POSIX.1-2001 here. man 7 inode says: > > > > The S_IF* constants are present in POSIX.1-2001 and later. > […] > The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but > both are present in POSIX.1-2001 > > POSIX specification != actual implementations. Also, Windows != POSIX, for example. See the gnulib page I pointed at. Also, looking through history with "git blame", etc. may find something. > Yes, because of man 2 open: > > > > EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR, > or includes O_CREAT without O_DIRECTORY. I assume you're on Linux, so "man 2" is the Linux Programmer's manual. But GDB works in other hosts too. So it may be the code was there for some other host. I mean, why did someone write that in the first place? Again, sounds like some code archaeology is in order. I forgot to say in the previous email, but it would be really nice if we could add some coverage for these change to the testsuite. I've asked before but I don't remember the answer -- Would it be possible to portably update e.g. gdb.base/fileio.exp to cover at least one kind of FILEIO_STDEV_SPECIAL file? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <6ea235bf-bc87-256a-e745-e54f5e97bf5c@farjump.io>]
* Re: [PATCH v3] Allow using special files with File I/O functions [not found] ` <6ea235bf-bc87-256a-e745-e54f5e97bf5c@farjump.io> @ 2018-06-29 14:40 ` Julio Guerra 2018-06-29 15:01 ` Pedro Alves 0 siblings, 1 reply; 9+ messages in thread From: Julio Guerra @ 2018-06-29 14:40 UTC (permalink / raw) To: Pedro Alves, gdb-patches Le 29/06/2018 à 16:28, Pedro Alves a écrit : > Hi, > > Your message came our hard to read: > > https://sourceware.org/ml/gdb-patches/2018-06/msg00715.html > > Please make sure your client is set up to quote replies. > > On 06/29/2018 03:01 PM, Julio Guerra wrote: > >> I assumed a system >= POSIX.1-2001 here. man 7 inode says: >> >> >> >> The S_IF* constants are present in POSIX.1-2001 and later. >> […] >> The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but >> both are present in POSIX.1-2001 >> >> > POSIX specification != actual implementations. Also, Windows != POSIX, > for example. See the gnulib page I pointed at. Also, looking through > history with "git blame", etc. may find something. Isn't GDB compiled with mingw? I am no expert in mingw, but I thought it was a POSIX implementation based on Windows. Anyway, we can add usual #ifdef guards arounds the cases. > >> Yes, because of man 2 open: >> >> >> >> EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR, >> or includes O_CREAT without O_DIRECTORY. > I assume you're on Linux, so "man 2" is the Linux Programmer's manual. > But GDB works in other hosts too. So it may be the code was there for > some other host. I mean, why did someone write that in the first place? > Again, sounds like some code archaeology is in order. Yes, but I checked it was POSIX too: http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html > > I forgot to say in the previous email, but it would be really nice if we > could add some coverage for these change to the testsuite. I've asked > before but I don't remember the answer -- Would it be possible to portably > update e.g. gdb.base/fileio.exp to cover at least one kind > of FILEIO_STDEV_SPECIAL file? Ok, I'll have a look, but I thought there File IOs were not implemented in gdbserver, so what runs this test? If it is natively run, it won't cover remote_fileio_func_*. -- Julio Guerra Co-founder & CTO of Farjump Mobile: +33 618 644 164 LinkedIn: https://linkedin.com/in/guerrajulio Slack: farjump.slack.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] Allow using special files with File I/O functions 2018-06-29 14:40 ` Julio Guerra @ 2018-06-29 15:01 ` Pedro Alves [not found] ` <817850f4-7ee1-6301-2256-a85b7a9edb02@farjump.io> 0 siblings, 1 reply; 9+ messages in thread From: Pedro Alves @ 2018-06-29 15:01 UTC (permalink / raw) To: Julio Guerra, gdb-patches On 06/29/2018 03:40 PM, Julio Guerra wrote: > Le 29/06/2018 à 16:28, Pedro Alves a écrit : >> On 06/29/2018 03:01 PM, Julio Guerra wrote: >> >>> I assumed a system >= POSIX.1-2001 here. man 7 inode says: >>> >>> >>> >>> The S_IF* constants are present in POSIX.1-2001 and later. >>> […] >>> The S_ISLNK() and S_ISSOCK() macros were not in POSIX.1-1996, but >>> both are present in POSIX.1-2001 >>> >>> >> POSIX specification != actual implementations. Also, Windows != POSIX, >> for example. See the gnulib page I pointed at. Also, looking through >> history with "git blame", etc. may find something. > > Isn't GDB compiled with mingw? I am no expert in mingw, but I thought it > was a POSIX implementation based on Windows. It's compiled with either MinGW or Cygwin. Cygwin is POSIX-like, but mingw is native Windows. > Anyway, we can add usual #ifdef guards arounds the cases. That may not be needed, but does not address the S_ISxxx vs S_IFxxx comment. Again, please take a look at the gnulib header. ISTM that S_ISxxx is always available (there's a default implementation that just returns 0), not sure about S_IFxxx. But please do take a better look. I've only skimmed it. >> >>> Yes, because of man 2 open: >>> >>> >>> >>> EISDIR: The named file is a directory and oflag includes O_WRONLY or O_RDWR, >>> or includes O_CREAT without O_DIRECTORY. >> I assume you're on Linux, so "man 2" is the Linux Programmer's manual. >> But GDB works in other hosts too. So it may be the code was there for >> some other host. I mean, why did someone write that in the first place? >> Again, sounds like some code archaeology is in order. > > Yes, but I checked it was POSIX too: > http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html Again, spec vs implementation. It may not be needed, but I wouldn't be surprised if that was needed on e.g., Windows either. Again, archaeology may help here. >> >> I forgot to say in the previous email, but it would be really nice if we >> could add some coverage for these change to the testsuite. I've asked >> before but I don't remember the answer -- Would it be possible to portably >> update e.g. gdb.base/fileio.exp to cover at least one kind >> of FILEIO_STDEV_SPECIAL file? > > Ok, I'll have a look, but I thought there File IOs were not implemented > in gdbserver, so what runs this test? If it is natively run, it won't > cover remote_fileio_func_*. You can run the testsuite against other remote stubs, not just gdbserver. Ideally, you'd set up the testsuite to against your stub. Did you try that? I'd recommend that regardless. Otherwise, even if we only test it when natively run, other folks that have embedded stubs that support fileio will end up exercising the test. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <817850f4-7ee1-6301-2256-a85b7a9edb02@farjump.io>]
* Re: [PATCH v3] Allow using special files with File I/O functions [not found] ` <817850f4-7ee1-6301-2256-a85b7a9edb02@farjump.io> @ 2018-07-04 9:44 ` Julio Guerra 2018-07-05 16:50 ` Pedro Alves 0 siblings, 1 reply; 9+ messages in thread From: Julio Guerra @ 2018-07-04 9:44 UTC (permalink / raw) To: Pedro Alves, gdb-patches Pedro, > You can run the testsuite against other remote stubs, not just gdbserver. > Ideally, you'd set up the testsuite to against your stub. Did you try > that? I'd recommend that regardless. > > Otherwise, even if we only test it when natively run, other folks that have > embedded stubs that support fileio will end up exercising the test. I have almost finished writing the tests, but I am struggling on how to create some special files: - Creating block or character devices require mknod with root privileges. - Creating a unix socket is not supported by the TCL library, and require extra tools (netcat, socat...) to create one from the command line. Here is what I use for now for the other types: - Link: TCL function `file link` - Regular: TCL function `open` - FIFO pipe command line: `mknod myfifo p` or `mkfifo myfifo` which doesn't exist on Windows. What do you suggest? Should I avoid every non portable test cases, which limits the tests only to links, regular files and directories? -- Julio Guerra ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] Allow using special files with File I/O functions 2018-07-04 9:44 ` Julio Guerra @ 2018-07-05 16:50 ` Pedro Alves 0 siblings, 0 replies; 9+ messages in thread From: Pedro Alves @ 2018-07-05 16:50 UTC (permalink / raw) To: Julio Guerra, gdb-patches On 07/04/2018 10:44 AM, Julio Guerra wrote: >> You can run the testsuite against other remote stubs, not just gdbserver. >> Ideally, you'd set up the testsuite to against your stub. Did you try >> that? I'd recommend that regardless. >> >> Otherwise, even if we only test it when natively run, other folks that have >> embedded stubs that support fileio will end up exercising the test. > > I have almost finished writing the tests, but I am struggling on how to > create some special files: > - Creating block or character devices require mknod with root privileges. > - Creating a unix socket is not supported by the TCL library, and > require extra tools (netcat, socat...) to create one from the command line. > > Here is what I use for now for the other types: > - Link: TCL function `file link` > - Regular: TCL function `open` > - FIFO pipe command line: `mknod myfifo p` or `mkfifo myfifo` which > doesn't exist on Windows. > > What do you suggest? Should I avoid every non portable test cases, which > limits the tests only to links, regular files and directories? Yeah, as long as we test at least one "special" file, I think it's OK. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-07-05 16:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20180628192635.44056-1-julio@farjump.io>
2018-06-28 19:27 ` [PATCH v3] Allow using special files with File I/O functions Julio Guerra
[not found] ` <b725e8cb-4a89-b204-1492-d8af5c76b89f@farjump.io>
2018-06-28 19:31 ` Julio Guerra
2018-06-29 13:42 ` Pedro Alves
[not found] ` <79758ca1-2541-9ae6-d793-b367d6094468@farjump.io>
2018-06-29 14:01 ` Julio Guerra
2018-06-29 14:28 ` Pedro Alves
[not found] ` <6ea235bf-bc87-256a-e745-e54f5e97bf5c@farjump.io>
2018-06-29 14:40 ` Julio Guerra
2018-06-29 15:01 ` Pedro Alves
[not found] ` <817850f4-7ee1-6301-2256-a85b7a9edb02@farjump.io>
2018-07-04 9:44 ` Julio Guerra
2018-07-05 16:50 ` Pedro Alves
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).