[v4] Allow using special files with File I/O functions
Commit Message
Remove the restriction in remote_fileio_func_open() to regular files only and
add support for special file types in the File IO stat structure.
The link file type is not part of the new definitions as stat() and fstat()
called by remote_fileio_func_stat() and remote_fileio_func_fstat() follow links,
it is thus not possible to obtain this file type in the File IO stat structure.
Add tests to cover as much cases as possible, limited by the fact that some
types such as FIFOs or character devices cannot be created on non-unix operating
systems. This limits the test cases to a regular file, a directory and the
standard output/input/error descriptors.
The major goal is to be able to write advanced embedded testing functions, like:
- using a FIFO between the embedded program and the host, instead of being
restricted to the GDB console only.
- mocking features based on host's by using some host device.
2018-07-05 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.
* 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.
* testsuite/gdb.base/fileio.c: Add test cases to cover some special
files and file descriptors.
* testsuite/gdb.base/fileio.exp: Likewise.
* doc/gdb.texinfo: Document the changes.
* NEWS: Briefly describe the changes of File I/O operations open, stat,
fstat.
Signed-off-by: Julio Guerra <julio@farjump.io>
---
gdb/ChangeLog | 16 ++++++
gdb/NEWS | 4 ++
gdb/common/fileio.c | 55 +++++++++++++++----
gdb/doc/gdb.texinfo | 7 ++-
gdb/remote-fileio.c | 10 +---
gdb/testsuite/gdb.base/fileio.c | 87 +++++++++++++++++++++++++++----
gdb/testsuite/gdb.base/fileio.exp | 26 ++++++++-
include/ChangeLog | 5 ++
include/gdb/fileio.h | 19 +++++--
9 files changed, 197 insertions(+), 32 deletions(-)
--
2.18.0
Comments
Hi Julio,
On 07/05/2018 10:16 AM, Julio Guerra wrote:
> Remove the restriction in remote_fileio_func_open() to regular files only and
> add support for special file types in the File IO stat structure.
> The link file type is not part of the new definitions as stat() and fstat()
> called by remote_fileio_func_stat() and remote_fileio_func_fstat() follow links,
> it is thus not possible to obtain this file type in the File IO stat structure.
>
> Add tests to cover as much cases as possible, limited by the fact that some
> types such as FIFOs or character devices cannot be created on non-unix operating
> systems. This limits the test cases to a regular file, a directory and the
> standard output/input/error descriptors.
>
> The major goal is to be able to write advanced embedded testing functions, like:
> - using a FIFO between the embedded program and the host, instead of being
> restricted to the GDB console only.
> - mocking features based on host's by using some host device.
>
> 2018-07-05 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.
> * 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.
> * testsuite/gdb.base/fileio.c: Add test cases to cover some special
> files and file descriptors.
> * testsuite/gdb.base/fileio.exp: Likewise.
> * doc/gdb.texinfo: Document the changes.
> * NEWS: Briefly describe the changes of File I/O operations open, stat,
> fstat.
Note that testsuite and docs have their own ChangeLogs. Please see:
https://sourceware.org/gdb/wiki/ContributionChecklist#ChangeLog
>
> Signed-off-by: Julio Guerra <julio@farjump.io>
I'm not sure whether I asked this before, but, just in case,
do you have a copyright assignment on file with the FSF?
I looked for one now and couldn't find it.
> ---
> gdb/ChangeLog | 16 ++++++
> gdb/NEWS | 4 ++
> gdb/common/fileio.c | 55 +++++++++++++++----
> gdb/doc/gdb.texinfo | 7 ++-
> gdb/remote-fileio.c | 10 +---
> gdb/testsuite/gdb.base/fileio.c | 87 +++++++++++++++++++++++++++----
> gdb/testsuite/gdb.base/fileio.exp | 26 ++++++++-
> include/ChangeLog | 5 ++
> include/gdb/fileio.h | 19 +++++--
> 9 files changed, 197 insertions(+), 32 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 7bd0cc4f95..b937f029e3 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,19 @@
> +2018-07-05 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.
> + * 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.
> + * testsuite/gdb.base/fileio.c: Add test cases to cover some special
> + files and file descriptors.
> + * testsuite/gdb.base/fileio.exp: Likewise.
> + * doc/gdb.texinfo: Document the changes.
> + * NEWS: Briefly describe the changes of File I/O operations open, stat,
> + fstat.
> +
> 2018-07-04 Tom Tromey <tom@tromey.com>
>
> * darwin-nat.c (darwin_attach_pid): Use exit_inferior.
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 2d1d161233..4a0ab4ac52 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -36,6 +36,10 @@
> * C expressions can now use _Alignof, and C++ expressions can now use
> alignof.
>
> +* The File I/O remote protocole extension now allows opening special files such
> + as FIFOs or character devices. Common file types are now be returned by stat
> + and fstat.
A couple typos in there "protocole", "are now be".
I suggest this:
* The File I/O remote protocol feature has been extended to allow
opening and stating special files such as FIFOs and character
devices. Previously only regular files and GDB's console were
supported.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 91ec219958..54b9ff6253 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -40986,7 +40986,8 @@ range of values.
> @table @code
>
> @item st_dev
> -A value of 0 represents a file, 1 the console.
> +A value of 0 represents a file, 1 GDB's console and 2 a special file
> +(@code{st_mode} gives further details on the file type).
I think:
- "0 represents a file"
+ "0 represents a regular file"
would help clarify this a bit.
>
> @item st_ino
> No valid meaning for the target. Transmitted unchanged.
> @@ -41073,8 +41074,12 @@ All values are given in hexadecimal representation.
> All values are given in octal representation.
>
> @smallexample
> + S_IFSOCK 0140000
> S_IFREG 0100000
> + S_IFBLK 060000
> S_IFDIR 040000
> + S_IFCHR 020000
> + S_IFIFO 010000
> S_IRUSR 0400
> S_IWUSR 0200
> S_IXUSR 0100
> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
> index 313da642ea..168590245e 100644
> --- a/gdb/remote-fileio.c
> +++ b/gdb/remote-fileio.c
> @@ -885,16 +885,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;
> - }
What happens if we stat/open some kind of unsupported file type?
Do we end up with st_mode == 0 and report success anyway, or is
something else catching it and returning FILEIO_EACCES or some such?
> diff --git a/gdb/testsuite/gdb.base/fileio.c b/gdb/testsuite/gdb.base/fileio.c
> index 7f482a34d3..c1902f5cc8 100644
> --- a/gdb/testsuite/gdb.base/fileio.c
> +++ b/gdb/testsuite/gdb.base/fileio.c
Can you say something about how you tested this?
I tried it here, and with the patch, the test as is fails with plain
"make check", i.e,. when testing with the native target:
$ make check TESTS="gdb.base/fileio.exp"
Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/fileio.exp ...
FAIL: gdb.base/fileio.exp: Stat a file
FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
FAIL: gdb.base/fileio.exp: Stat a directory
FAIL: gdb.base/fileio.exp: Fstat an open file
FAIL: gdb.base/fileio.exp: Fstat a directory
FAIL: gdb.base/fileio.exp: Fstat standard output
FAIL: gdb.base/fileio.exp: Fstat standard input
FAIL: gdb.base/fileio.exp: Fstat standard error
We have to make sure that continues passing, as that helps
ensure the testcase's program is written in a portable fashion.
I think part of it is that the testcase is now assuming st_dev
contains the File I/O 0/1/2 values, for regular/console/special.
I think the best course of action here is to add something like
this:
static int
is_st_dev (int actual, int expected)
{
#ifdef TESTING_RSP
return actual == expected;
#else
return 1;
#endif
}
With the .exp file passing additional_flags=-DTESTING_RSP if testing
against a remote protocol target.
And then use it like:
printf ("fstat 3: ret = %d, errno = %d %s\n", ret, errno,
- st.st_dev == 2 && S_ISDIR (st.st_mode) ? "OK" : "");
+ is_st_dev (st.st_dev, 2) && S_ISDIR (st.st_mode) ? "OK" : "");
The .exp can tell when we're testing against a remote protocol
target with:
if {[target_info gdb_protocol] == "remote"
|| [target_info gdb_protocol] == "extended-remote"} {
>
> #define STRING "Hello World"
>
> @@ -292,7 +301,8 @@ test_stat (void)
> ret = stat (OUTDIR FILENAME, &st);
> if (!ret)
> printf ("stat 1: ret = %d, errno = %d %s\n", ret, errno,
> - st.st_size == 11 ? "OK" : "");
> + st.st_dev == 0 && S_ISREG (st.st_mode) && st.st_size == 11 ?
> + "OK" : "");
> else
> printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
> stop ();
> @@ -311,8 +321,20 @@ test_stat (void)
> /* Nonexistant file */
> errno = 0;
> ret = stat (NONEXISTANT, &st);
> - printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
> - strerrno (errno));
> + if (!ret)
> + printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
> + strerrno (errno));
> + else
> + printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
Do we want to print errno in the !ret case? That indicates the
stat call succeeded (even though we expect it to fail).
Might be that helps simplify the .exp, I haven't checked.
But at least the strerrno call should be on the else
branch, as that's the branch where errno is meaninful, and
the .exp expects it:
FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
Did this pass against your stub?
Also, here both branches should say "stat 4", I think?
> + stop ();
> + /* Special file: directory */
> + errno = 0;
> + ret = stat (OUTDIR DIRECTORY, &st);
> + if (!ret)
> + printf ("stat 5: ret = %d, errno = %d %s\n", ret, errno,
> + st.st_dev == 2 && S_ISDIR (st.st_mode) ? "OK" : "");
> + else
> + printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
Shouldn't both branches here say "stat 5" ?
> void
> diff --git a/gdb/testsuite/gdb.base/fileio.exp b/gdb/testsuite/gdb.base/fileio.exp
> index bc409c26aa..234f304ac7 100644
> --- a/gdb/testsuite/gdb.base/fileio.exp
> +++ b/gdb/testsuite/gdb.base/fileio.exp
> @@ -31,7 +31,7 @@ if {[is_remote host]} {
>
> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> executable \
> - [list debug "additional_flags=-DOUTDIR=\"$outdir/\""]] != "" } {
> + [list debug "additional_flags=-DOUTDIR=\"$outdir/\" [target_info fileio,cflags]" "ldflags=[target_info fileio,ldflags]"]] != "" } {
I couldn't tell what's this change for? Why did you need it?
> untested "failed to compile"
> return -1
> }
> @@ -136,6 +136,10 @@ gdb_test continue \
> "Continuing\\..*close 2:.*EBADF$stop_msg" \
> "Closing an invalid file descriptor returns EBADF"
>
> +# Prepare some different file types for stat and fstat.
> +set filetype_directory [file join $outdir "directory.fileio.test"]
> +file mkdir $filetype_directory
> +
Please use remote_exec like the other mkdir calls in the file.
Thanks,
Pedro Alves
>> Remove the restriction in remote_fileio_func_open() to regular files only and
>> add support for special file types in the File IO stat structure.
>> The link file type is not part of the new definitions as stat() and fstat()
>> called by remote_fileio_func_stat() and remote_fileio_func_fstat() follow links,
>> it is thus not possible to obtain this file type in the File IO stat structure.
>>
>> Add tests to cover as much cases as possible, limited by the fact that some
>> types such as FIFOs or character devices cannot be created on non-unix operating
>> systems. This limits the test cases to a regular file, a directory and the
>> standard output/input/error descriptors.
>>
>> The major goal is to be able to write advanced embedded testing functions, like:
>> - using a FIFO between the embedded program and the host, instead of being
>> restricted to the GDB console only.
>> - mocking features based on host's by using some host device.
>>
>> 2018-07-05 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.
>> * 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.
>> * testsuite/gdb.base/fileio.c: Add test cases to cover some special
>> files and file descriptors.
>> * testsuite/gdb.base/fileio.exp: Likewise.
>> * doc/gdb.texinfo: Document the changes.
>> * NEWS: Briefly describe the changes of File I/O operations open, stat,
>> fstat.
> Note that testsuite and docs have their own ChangeLogs. Please see:
>
> https://sourceware.org/gdb/wiki/ContributionChecklist#ChangeLog
Ok.
>
>> Signed-off-by: Julio Guerra <julio@farjump.io>
> I'm not sure whether I asked this before, but, just in case,
> do you have a copyright assignment on file with the FSF?
> I looked for one now and couldn't find it.
>
No, I don't.
>> ---
>> gdb/ChangeLog | 16 ++++++
>> gdb/NEWS | 4 ++
>> gdb/common/fileio.c | 55 +++++++++++++++----
>> gdb/doc/gdb.texinfo | 7 ++-
>> gdb/remote-fileio.c | 10 +---
>> gdb/testsuite/gdb.base/fileio.c | 87 +++++++++++++++++++++++++++----
>> gdb/testsuite/gdb.base/fileio.exp | 26 ++++++++-
>> include/ChangeLog | 5 ++
>> include/gdb/fileio.h | 19 +++++--
>> 9 files changed, 197 insertions(+), 32 deletions(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 7bd0cc4f95..b937f029e3 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,19 @@
>> +2018-07-05 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.
>> + * 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.
>> + * testsuite/gdb.base/fileio.c: Add test cases to cover some special
>> + files and file descriptors.
>> + * testsuite/gdb.base/fileio.exp: Likewise.
>> + * doc/gdb.texinfo: Document the changes.
>> + * NEWS: Briefly describe the changes of File I/O operations open, stat,
>> + fstat.
>> +
>> 2018-07-04 Tom Tromey <tom@tromey.com>
>>
>> * darwin-nat.c (darwin_attach_pid): Use exit_inferior.
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 2d1d161233..4a0ab4ac52 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -36,6 +36,10 @@
>> * C expressions can now use _Alignof, and C++ expressions can now use
>> alignof.
>>
>> +* The File I/O remote protocole extension now allows opening special files such
>> + as FIFOs or character devices. Common file types are now be returned by stat
>> + and fstat.
> A couple typos in there "protocole", "are now be".
>
>
> I suggest this:
>
> * The File I/O remote protocol feature has been extended to allow
> opening and stating special files such as FIFOs and character
> devices. Previously only regular files and GDB's console were
> supported.
>
Ok.
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index 91ec219958..54b9ff6253 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -40986,7 +40986,8 @@ range of values.
>> @table @code
>>
>> @item st_dev
>> -A value of 0 represents a file, 1 the console.
>> +A value of 0 represents a file, 1 GDB's console and 2 a special file
>> +(@code{st_mode} gives further details on the file type).
> I think:
>
> - "0 represents a file"
> + "0 represents a regular file"
>
> would help clarify this a bit.
Ok.
>>
>> @item st_ino
>> No valid meaning for the target. Transmitted unchanged.
>> @@ -41073,8 +41074,12 @@ All values are given in hexadecimal representation.
>> All values are given in octal representation.
>>
>> @smallexample
>> + S_IFSOCK 0140000
>> S_IFREG 0100000
>> + S_IFBLK 060000
>> S_IFDIR 040000
>> + S_IFCHR 020000
>> + S_IFIFO 010000
>> S_IRUSR 0400
>> S_IWUSR 0200
>> S_IXUSR 0100
>> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
>> index 313da642ea..168590245e 100644
>> --- a/gdb/remote-fileio.c
>> +++ b/gdb/remote-fileio.c
>> @@ -885,16 +885,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;
>> - }
>
> What happens if we stat/open some kind of unsupported file type?
> Do we end up with st_mode == 0 and report success anyway, or is
> something else catching it and returning FILEIO_EACCES or some such?
>
Yes, bits SFMT of st_mode end up with everything 0 and it doesn't fail.
It's like not knowing what kind of file it exactly is, but still get
other values.
>> diff --git a/gdb/testsuite/gdb.base/fileio.c b/gdb/testsuite/gdb.base/fileio.c
>> index 7f482a34d3..c1902f5cc8 100644
>> --- a/gdb/testsuite/gdb.base/fileio.c
>> +++ b/gdb/testsuite/gdb.base/fileio.c
> Can you say something about how you tested this?
>
> I tried it here, and with the patch, the test as is fails with plain
> "make check", i.e,. when testing with the native target:
>
> $ make check TESTS="gdb.base/fileio.exp"
> Running /home/pedro/gdb/binutils-gdb/src/gdb/testsuite/gdb.base/fileio.exp ...
> FAIL: gdb.base/fileio.exp: Stat a file
> FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
> FAIL: gdb.base/fileio.exp: Stat a directory
> FAIL: gdb.base/fileio.exp: Fstat an open file
> FAIL: gdb.base/fileio.exp: Fstat a directory
> FAIL: gdb.base/fileio.exp: Fstat standard output
> FAIL: gdb.base/fileio.exp: Fstat standard input
> FAIL: gdb.base/fileio.exp: Fstat standard error
>
> We have to make sure that continues passing, as that helps
> ensure the testcase's program is written in a portable fashion.
>
> I think part of it is that the testcase is now assuming st_dev
> contains the File I/O 0/1/2 values, for regular/console/special.
>
> I think the best course of action here is to add something like
> this:
>
> static int
> is_st_dev (int actual, int expected)
> {
> #ifdef TESTING_RSP
> return actual == expected;
> #else
> return 1;
> #endif
> }
>
> With the .exp file passing additional_flags=-DTESTING_RSP if testing
> against a remote protocol target.
>
> And then use it like:
>
> printf ("fstat 3: ret = %d, errno = %d %s\n", ret, errno,
> - st.st_dev == 2 && S_ISDIR (st.st_mode) ? "OK" : "");
> + is_st_dev (st.st_dev, 2) && S_ISDIR (st.st_mode) ? "OK" : "");
>
>
> The .exp can tell when we're testing against a remote protocol
> target with:
>
> if {[target_info gdb_protocol] == "remote"
> || [target_info gdb_protocol] == "extended-remote"} {
Ok. I see. I tested with our stubs running on a Raspberry Pi by defining
a board .exp file, inspired by board definitions sid and gdbserver.
>>
>> #define STRING "Hello World"
>>
>> @@ -292,7 +301,8 @@ test_stat (void)
>> ret = stat (OUTDIR FILENAME, &st);
>> if (!ret)
>> printf ("stat 1: ret = %d, errno = %d %s\n", ret, errno,
>> - st.st_size == 11 ? "OK" : "");
>> + st.st_dev == 0 && S_ISREG (st.st_mode) && st.st_size == 11 ?
>> + "OK" : "");
>> else
>> printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
>> stop ();
>> @@ -311,8 +321,20 @@ test_stat (void)
>> /* Nonexistant file */
>> errno = 0;
>> ret = stat (NONEXISTANT, &st);
>> - printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>> - strerrno (errno));
>> + if (!ret)
>> + printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>> + strerrno (errno));
>> + else
>> + printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
> Do we want to print errno in the !ret case? That indicates the
> stat call succeeded (even though we expect it to fail).
> Might be that helps simplify the .exp, I haven't checked.
>
> But at least the strerrno call should be on the else
> branch, as that's the branch where errno is meaninful, and
> the .exp expects it:
>
> FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
>
> Did this pass against your stub?
Yes, here are logs of GDB's execution on this case:
> Continuing.
> stat 4: ret = -1, errno = 2 ENOENT
>
> Breakpoint 2, stop () at
/home/julio/gnu/gdb/build-arm-none-eabi/gdb/testsuite/../../../gdb/testsuite/gdb.base/fileio.c:84
> 84 static void stop (void) {}
>
> Also, here both branches should say "stat 4", I think?
>
>
>> + stop ();
>> + /* Special file: directory */
>> + errno = 0;
>> + ret = stat (OUTDIR DIRECTORY, &st);
>> + if (!ret)
>> + printf ("stat 5: ret = %d, errno = %d %s\n", ret, errno,
>> + st.st_dev == 2 && S_ISDIR (st.st_mode) ? "OK" : "");
>> + else
>> + printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
> Shouldn't both branches here say "stat 5" ?
Excessive copy/paste, sorry.
>> void
>> diff --git a/gdb/testsuite/gdb.base/fileio.exp b/gdb/testsuite/gdb.base/fileio.exp
>> index bc409c26aa..234f304ac7 100644
>> --- a/gdb/testsuite/gdb.base/fileio.exp
>> +++ b/gdb/testsuite/gdb.base/fileio.exp
>> @@ -31,7 +31,7 @@ if {[is_remote host]} {
>>
>> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>> executable \
>> - [list debug "additional_flags=-DOUTDIR=\"$outdir/\""]] != "" } {
>> + [list debug "additional_flags=-DOUTDIR=\"$outdir/\" [target_info fileio,cflags]" "ldflags=[target_info fileio,ldflags]"]] != "" } {
> I couldn't tell what's this change for? Why did you need it?
I couldn't find any other way of adding some CFLAGS and LDFLAGS to the
call to the cross-compiler to link against the libc using File IOs, to
add target-specific compilation flags, etc. For example, in my case:
> set_board_info fileio,cflags "--specs=$sdk/Alpha.specs
-mfloat-abi=hard -mfpu=vfp -march=armv6zk -mtune=arm1176jzf-s"
> set_board_info fileio,ldflags "-Wl,-T$sdk/link.ld"
>
>> untested "failed to compile"
>> return -1
>> }
>> @@ -136,6 +136,10 @@ gdb_test continue \
>> "Continuing\\..*close 2:.*EBADF$stop_msg" \
>> "Closing an invalid file descriptor returns EBADF"
>>
>> +# Prepare some different file types for stat and fstat.
>> +set filetype_directory [file join $outdir "directory.fileio.test"]
>> +file mkdir $filetype_directory
>> +
> Please use remote_exec like the other mkdir calls in the file.
>
Ok.
--
Julio Guerra
Co-founder & CTO of Farjump
Mobile: +33 618 644 164
LinkedIn: https://linkedin.com/in/guerrajulio
Slack: farjump.slack.com
On 07/09/2018 02:16 PM, Julio Guerra wrote:
>>> Remove the restriction in remote_fileio_func_open() to regular files only and
>>> add support for special file types in the File IO stat structure.
>>> The link file type is not part of the new definitions as stat() and fstat()
>>> called by remote_fileio_func_stat() and remote_fileio_func_fstat() follow links,
>>> it is thus not possible to obtain this file type in the File IO stat structure.
>>>
>>> Add tests to cover as much cases as possible, limited by the fact that some
>>> types such as FIFOs or character devices cannot be created on non-unix operating
>>> systems. This limits the test cases to a regular file, a directory and the
>>> standard output/input/error descriptors.
>>>
>>> The major goal is to be able to write advanced embedded testing functions, like:
>>> - using a FIFO between the embedded program and the host, instead of being
>>> restricted to the GDB console only.
>>> - mocking features based on host's by using some host device.
>>>
>>> 2018-07-05 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.
>>> * 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.
>>> * testsuite/gdb.base/fileio.c: Add test cases to cover some special
>>> files and file descriptors.
>>> * testsuite/gdb.base/fileio.exp: Likewise.
>>> * doc/gdb.texinfo: Document the changes.
>>> * NEWS: Briefly describe the changes of File I/O operations open, stat,
>>> fstat.
>> Note that testsuite and docs have their own ChangeLogs. Please see:
>>
>> https://sourceware.org/gdb/wiki/ContributionChecklist#ChangeLog
>
> Ok.
>
>>
>>> Signed-off-by: Julio Guerra <julio@farjump.io>
>> I'm not sure whether I asked this before, but, just in case,
>> do you have a copyright assignment on file with the FSF?
>> I looked for one now and couldn't find it.
>>
>
> No, I don't.
See:
https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
The request-assign.future one is most common one. Please follow the
instructions at the top of the file.
>>> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
>>> index 313da642ea..168590245e 100644
>>> --- a/gdb/remote-fileio.c
>>> +++ b/gdb/remote-fileio.c
>>> @@ -885,16 +885,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;
>>> - }
>>
>> What happens if we stat/open some kind of unsupported file type?
>> Do we end up with st_mode == 0 and report success anyway, or is
>> something else catching it and returning FILEIO_EACCES or some such?
>>
>
> Yes, bits SFMT of st_mode end up with everything 0 and it doesn't fail.
> It's like not knowing what kind of file it exactly is, but still get
> other values.
Hmm, OK. I mildly worry whether that that might cause trouble.
I wonder what other filesystem network protocols do here. Like,
e.g., nfs, sshfs, etc.
>>>
>>> #define STRING "Hello World"
>>>
>>> @@ -292,7 +301,8 @@ test_stat (void)
>>> ret = stat (OUTDIR FILENAME, &st);
>>> if (!ret)
>>> printf ("stat 1: ret = %d, errno = %d %s\n", ret, errno,
>>> - st.st_size == 11 ? "OK" : "");
>>> + st.st_dev == 0 && S_ISREG (st.st_mode) && st.st_size == 11 ?
>>> + "OK" : "");
>>> else
>>> printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
>>> stop ();
>>> @@ -311,8 +321,20 @@ test_stat (void)
>>> /* Nonexistant file */
>>> errno = 0;
>>> ret = stat (NONEXISTANT, &st);
>>> - printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>>> - strerrno (errno));
>>> + if (!ret)
>>> + printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>>> + strerrno (errno));
>>> + else
>>> + printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
>> Do we want to print errno in the !ret case? That indicates the
>> stat call succeeded (even though we expect it to fail).
>> Might be that helps simplify the .exp, I haven't checked.
>>
>> But at least the strerrno call should be on the else
>> branch, as that's the branch where errno is meaninful, and
>> the .exp expects it:
>>
>> FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
>>
>> Did this pass against your stub?
>
> Yes, here are logs of GDB's execution on this case:
>
>> Continuing.
>> stat 4: ret = -1, errno = 2 ENOENT
I don't see how that could have been logs for the patch you sent,
because if ret really was -1, then that would have said "stat 1",
and would not have printed ENOENT. Maybe you tested a
different patch?
>>> diff --git a/gdb/testsuite/gdb.base/fileio.exp b/gdb/testsuite/gdb.base/fileio.exp
>>> index bc409c26aa..234f304ac7 100644
>>> --- a/gdb/testsuite/gdb.base/fileio.exp
>>> +++ b/gdb/testsuite/gdb.base/fileio.exp
>>> @@ -31,7 +31,7 @@ if {[is_remote host]} {
>>>
>>> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>> executable \
>>> - [list debug "additional_flags=-DOUTDIR=\"$outdir/\""]] != "" } {
>>> + [list debug "additional_flags=-DOUTDIR=\"$outdir/\" [target_info fileio,cflags]" "ldflags=[target_info fileio,ldflags]"]] != "" } {
>> I couldn't tell what's this change for? Why did you need it?
>
> I couldn't find any other way of adding some CFLAGS and LDFLAGS to the
> call to the cross-compiler to link against the libc using File IOs, to
> add target-specific compilation flags, etc. For example, in my case:
>
>> set_board_info fileio,cflags "--specs=$sdk/Alpha.specs
> -mfloat-abi=hard -mfpu=vfp -march=armv6zk -mtune=arm1176jzf-s"
>> set_board_info fileio,ldflags "-Wl,-T$sdk/link.ld"
Is this something really specific to this testcase? Don't you
need to do the same for all other testcases?
Did you try CC_FOR_TARGET/LD_FOR_TARGET?
Thanks,
Pedro Alves
>>>> Signed-off-by: Julio Guerra <julio@farjump.io>
>>> I'm not sure whether I asked this before, but, just in case,
>>> do you have a copyright assignment on file with the FSF?
>>> I looked for one now and couldn't find it.
>>>
>> No, I don't.
> See:
>
> https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
>
> The request-assign.future one is most common one. Please follow the
> instructions at the top of the file.
Done.
>
>>>> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
>>>> index 313da642ea..168590245e 100644
>>>> --- a/gdb/remote-fileio.c
>>>> +++ b/gdb/remote-fileio.c
>>>> @@ -885,16 +885,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;
>>>> - }
>>> What happens if we stat/open some kind of unsupported file type?
>>> Do we end up with st_mode == 0 and report success anyway, or is
>>> something else catching it and returning FILEIO_EACCES or some such?
>>>
>> Yes, bits SFMT of st_mode end up with everything 0 and it doesn't fail.
>> It's like not knowing what kind of file it exactly is, but still get
>> other values.
> Hmm, OK. I mildly worry whether that that might cause trouble.
> I wonder what other filesystem network protocols do here. Like,
> e.g., nfs, sshfs, etc.
>
The 3 options I see:
1 - No restrinction: this one.
2 - Warning: test SFMT bits and when "unsupported", set File IO SFMT
bits to some specific FILEIO_S_UNKNOWN value.
3 - Restrict: strictly restrict to those SFMT types I added.
I would go for 1 or 2, to avoid another similar restriction like there
was in open before this patch.
>>>> /* Nonexistant file */
>>>> errno = 0;
>>>> ret = stat (NONEXISTANT, &st);
>>>> - printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>>>> - strerrno (errno));
>>>> + if (!ret)
>>>> + printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
>>>> + strerrno (errno));
>>>> + else
>>>> + printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
>>> Do we want to print errno in the !ret case? That indicates the
>>> stat call succeeded (even though we expect it to fail).
>>> Might be that helps simplify the .exp, I haven't checked.
>>>
>>> But at least the strerrno call should be on the else
>>> branch, as that's the branch where errno is meaninful, and
>>> the .exp expects it:
>>>
>>> FAIL: gdb.base/fileio.exp: Stat a nonexistant file returns ENOENT
>>>
>>> Did this pass against your stub?
>> Yes, here are logs of GDB's execution on this case:
>>
>>> Continuing.
>>> stat 4: ret = -1, errno = 2 ENOENT
> I don't see how that could have been logs for the patch you sent,
> because if ret really was -1, then that would have said "stat 1",
> and would not have printed ENOENT. Maybe you tested a
> different patch?
I guess I didn't run the tests again after this last 'if ... else'
modification, sorry...
>>>> diff --git a/gdb/testsuite/gdb.base/fileio.exp b/gdb/testsuite/gdb.base/fileio.exp
>>>> index bc409c26aa..234f304ac7 100644
>>>> --- a/gdb/testsuite/gdb.base/fileio.exp
>>>> +++ b/gdb/testsuite/gdb.base/fileio.exp
>>>> @@ -31,7 +31,7 @@ if {[is_remote host]} {
>>>>
>>>> if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
>>>> executable \
>>>> - [list debug "additional_flags=-DOUTDIR=\"$outdir/\""]] != "" } {
>>>> + [list debug "additional_flags=-DOUTDIR=\"$outdir/\" [target_info fileio,cflags]" "ldflags=[target_info fileio,ldflags]"]] != "" } {
>>> I couldn't tell what's this change for? Why did you need it?
>> I couldn't find any other way of adding some CFLAGS and LDFLAGS to the
>> call to the cross-compiler to link against the libc using File IOs, to
>> add target-specific compilation flags, etc. For example, in my case:
>>
>>> set_board_info fileio,cflags "--specs=$sdk/Alpha.specs
>> -mfloat-abi=hard -mfpu=vfp -march=armv6zk -mtune=arm1176jzf-s"
>>> set_board_info fileio,ldflags "-Wl,-T$sdk/link.ld"
> Is this something really specific to this testcase? Don't you
> need to do the same for all other testcases?
Yes, every other tests involving cross-compilation and the C library
need the same.
> Did you try CC_FOR_TARGET/LD_FOR_TARGET?
I just tried and couldn't make it work. Common use cases are indeed
adding extra flags but also extra files to be linked (such as the fileio
open, read, write, ... stubs).
--
Julio Guerra
On 07/09/2018 04:22 PM, Julio Guerra wrote:
>
>>>>> Signed-off-by: Julio Guerra <julio@farjump.io>
>>>> I'm not sure whether I asked this before, but, just in case,
>>>> do you have a copyright assignment on file with the FSF?
>>>> I looked for one now and couldn't find it.
>>>>
>>> No, I don't.
>> See:
>>
>> https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
>>
>> The request-assign.future one is most common one. Please follow the
>> instructions at the top of the file.
>
> Done.
Thanks.
>
>>
>>>>> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
>>>>> index 313da642ea..168590245e 100644
>>>>> --- a/gdb/remote-fileio.c
>>>>> +++ b/gdb/remote-fileio.c
>>>>> @@ -885,16 +885,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;
>>>>> - }
>>>> What happens if we stat/open some kind of unsupported file type?
>>>> Do we end up with st_mode == 0 and report success anyway, or is
>>>> something else catching it and returning FILEIO_EACCES or some such?
>>>>
>>> Yes, bits SFMT of st_mode end up with everything 0 and it doesn't fail.
>>> It's like not knowing what kind of file it exactly is, but still get
>>> other values.
>> Hmm, OK. I mildly worry whether that that might cause trouble.
>> I wonder what other filesystem network protocols do here. Like,
>> e.g., nfs, sshfs, etc.
>>
>
> The 3 options I see:
> 1 - No restrinction: this one.
> 2 - Warning: test SFMT bits and when "unsupported", set File IO SFMT
> bits to some specific FILEIO_S_UNKNOWN value.
> 3 - Restrict: strictly restrict to those SFMT types I added.
>
> I would go for 1 or 2, to avoid another similar restriction like there
> was in open before this patch.
OK, let's try going with 1. But please add a comment to the
effect somewhere so that it doesn't looks like it happens by accident.
>>>> I couldn't tell what's this change for? Why did you need it?
>>> I couldn't find any other way of adding some CFLAGS and LDFLAGS to the
>>> call to the cross-compiler to link against the libc using File IOs, to
>>> add target-specific compilation flags, etc. For example, in my case:
>>>
>>>> set_board_info fileio,cflags "--specs=$sdk/Alpha.specs
>>> -mfloat-abi=hard -mfpu=vfp -march=armv6zk -mtune=arm1176jzf-s"
>>>> set_board_info fileio,ldflags "-Wl,-T$sdk/link.ld"
>> Is this something really specific to this testcase? Don't you
>> need to do the same for all other testcases?
>
> Yes, every other tests involving cross-compilation and the C library
> need the same.
>
>> Did you try CC_FOR_TARGET/LD_FOR_TARGET?
>
> I just tried and couldn't make it work.
That's strange, because we have a few boards that rely on
those. Grep for CC_FOR_TARGET under src/gdb/testsuite/boards/.
I actually meant to suggest CFLAGS_FOR_TARGET/LDFLAGS_FOR_TARGET,
but I don't think there's a practical difference, just like
e.g., "CC=gcc -m32" vs "CFLAGS=-m32" should end up the same in
practice.
> Common use cases are indeed
> adding extra flags but also extra files to be linked (such as the fileio
> open, read, write, ... stubs).
Thanks,
Pedro Alves
@@ -1,3 +1,19 @@
+2018-07-05 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.
+ * 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.
+ * testsuite/gdb.base/fileio.c: Add test cases to cover some special
+ files and file descriptors.
+ * testsuite/gdb.base/fileio.exp: Likewise.
+ * doc/gdb.texinfo: Document the changes.
+ * NEWS: Briefly describe the changes of File I/O operations open, stat,
+ fstat.
+
2018-07-04 Tom Tromey <tom@tromey.com>
* darwin-nat.c (darwin_attach_pid): Use exit_inferior.
@@ -36,6 +36,10 @@
* C expressions can now use _Alignof, and C++ expressions can now use
alignof.
+* The File I/O remote protocole extension now allows opening special files such
+ as FIFOs or character devices. Common file types are now be returned by stat
+ and fstat.
+
* New commands
set debug fbsd-nat
@@ -119,12 +119,40 @@ 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)
+ {
+#ifdef S_IFSOCK
+ case FILEIO_S_IFSOCK:
+ *mode_p |= S_IFSOCK;
+ break;
+#endif
+#ifdef S_IFREG
+ case FILEIO_S_IFREG:
+ mode |= S_IFREG;
+ break;
+#endif
+#ifdef S_IFBLK
+ case FILEIO_S_IFBLK:
+ mode |= S_IFBLK;
+ break;
+#endif
+#ifdef S_IFDIR
+ case FILEIO_S_IFDIR:
+ mode |= S_IFDIR;
+ break;
+#endif
+#ifdef S_IFCHR
+ case FILEIO_S_IFCHR:
+ mode |= S_IFCHR;
+ break;
+#endif
+#ifdef S_IFIFO
+ case FILEIO_S_IFIFO:
+ mode |= S_IFIFO;
+ break;
+#endif
+ }
+
if (fileio_mode & FILEIO_S_IRUSR)
mode |= S_IRUSR;
if (fileio_mode & FILEIO_S_IWUSR)
@@ -167,10 +195,17 @@ fileio_mode_pack (mode_t mode)
if (S_ISREG (mode))
tmode |= FILEIO_S_IFREG;
- if (S_ISDIR (mode))
+ else if (S_ISDIR (mode))
tmode |= FILEIO_S_IFDIR;
- if (S_ISCHR (mode))
+ else if (S_ISCHR (mode))
tmode |= FILEIO_S_IFCHR;
+ else if (S_ISSOCK (mode))
+ tmode |= FILEIO_S_IFSOCK;
+ else if (S_ISBLK (mode))
+ tmode |= FILEIO_S_IFBLK;
+ else if (S_ISFIFO (mode))
+ tmode |= FILEIO_S_IFIFO;
+
if (mode & S_IRUSR)
tmode |= FILEIO_S_IRUSR;
if (mode & S_IWUSR)
@@ -224,8 +259,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);
@@ -40986,7 +40986,8 @@ range of values.
@table @code
@item st_dev
-A value of 0 represents a file, 1 the console.
+A value of 0 represents a file, 1 GDB's console and 2 a special file
+(@code{st_mode} gives further details on the file type).
@item st_ino
No valid meaning for the target. Transmitted unchanged.
@@ -41073,8 +41074,12 @@ All values are given in hexadecimal representation.
All values are given in octal representation.
@smallexample
+ S_IFSOCK 0140000
S_IFREG 0100000
+ S_IFBLK 060000
S_IFDIR 040000
+ S_IFCHR 020000
+ S_IFIFO 010000
S_IRUSR 0400
S_IWUSR 0200
S_IXUSR 0100
@@ -885,16 +885,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 +932,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 +964,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)
@@ -32,11 +32,19 @@ close(int fd);
1) Attempt to close an invalid file descriptor - EBADF
stat(const char *file_name, struct stat *buf);
-1) Pathname is a null string - ENOENT
-2) Pathname does not exist - ENOENT
+1) Regular file
+2) Pathname is a null string - ENOENT
+3) Pathname is an empty string - ENOENT or EFAULT
+4) Pathname does not exist - ENOENT
+5) Directory
fstat(int filedes, struct stat *buf);
-1) Attempt to stat using an invalid file descriptor - EBADF
+1) Regular file
+2) Attempt to stat using an invalid file descriptor - EBADF
+3) Directory
+4) Standard input
+5) Standard output
+6) Standard error
isatty (int desc);
Not applicable. We will test that it returns 1 when expected and a case
@@ -68,9 +76,10 @@ static const char *strerrno (int err);
#define RENAMED "bar.fileio.test"
#define NONEXISTANT "nofoo.fileio.test"
#define NOWRITE "nowrt.fileio.test"
-#define TESTDIR1 "dir1.fileio.test"
-#define TESTDIR2 "dir2.fileio.test"
-#define TESTSUBDIR "dir1.fileio.test/subdir.fileio.test"
+#define TESTDIR1 "dir1.fileio.test"
+#define TESTDIR2 "dir2.fileio.test"
+#define TESTSUBDIR "dir1.fileio.test/subdir.fileio.test"
+#define DIRECTORY "directory.fileio.test"
#define STRING "Hello World"
@@ -292,7 +301,8 @@ test_stat (void)
ret = stat (OUTDIR FILENAME, &st);
if (!ret)
printf ("stat 1: ret = %d, errno = %d %s\n", ret, errno,
- st.st_size == 11 ? "OK" : "");
+ st.st_dev == 0 && S_ISREG (st.st_mode) && st.st_size == 11 ?
+ "OK" : "");
else
printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
stop ();
@@ -311,8 +321,20 @@ test_stat (void)
/* Nonexistant file */
errno = 0;
ret = stat (NONEXISTANT, &st);
- printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
- strerrno (errno));
+ if (!ret)
+ printf ("stat 4: ret = %d, errno = %d %s\n", ret, errno,
+ strerrno (errno));
+ else
+ printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
+ stop ();
+ /* Special file: directory */
+ errno = 0;
+ ret = stat (OUTDIR DIRECTORY, &st);
+ if (!ret)
+ printf ("stat 5: ret = %d, errno = %d %s\n", ret, errno,
+ st.st_dev == 2 && S_ISDIR (st.st_mode) ? "OK" : "");
+ else
+ printf ("stat 1: ret = %d, errno = %d\n", ret, errno);
stop ();
}
@@ -331,7 +353,8 @@ test_fstat (void)
ret = fstat (fd, &st);
if (!ret)
printf ("fstat 1: ret = %d, errno = %d %s\n", ret, errno,
- st.st_size == 11 ? "OK" : "");
+ st.st_dev == 0 && S_ISREG (st.st_mode) && st.st_size == 11 ?
+ "OK" : "");
else
printf ("fstat 1: ret = %d, errno = %d\n", ret, errno);
close (fd);
@@ -345,6 +368,50 @@ test_fstat (void)
printf ("fstat 2: ret = %d, errno = %d %s\n", ret, errno,
strerrno (errno));
stop ();
+ /* Special file: directory */
+ errno = 0;
+ fd = open (OUTDIR DIRECTORY, O_RDONLY);
+ if (fd >= 0)
+ {
+ errno = 0;
+ ret = fstat (fd, &st);
+ if (!ret)
+ printf ("fstat 3: ret = %d, errno = %d %s\n", ret, errno,
+ st.st_dev == 2 && S_ISDIR (st.st_mode) ? "OK" : "");
+ else
+ printf ("fstat 3: ret = %d, errno = %d\n", ret, errno);
+ close (fd);
+ }
+ else
+ printf ("fstat 3: errno = %d\n", errno);
+ stop ();
+ /* Standard input */
+ errno = 0;
+ ret = fstat (STDIN_FILENO, &st);
+ if (!ret)
+ printf ("fstat 4: ret = %d, errno = %d %s\n", ret, errno,
+ st.st_dev == 1 ? "OK" : "");
+ else
+ printf ("fstat 4: ret = %d, errno = %d\n", ret, errno);
+ stop ();
+ /* Standard output */
+ errno = 0;
+ ret = fstat (STDOUT_FILENO, &st);
+ if (!ret)
+ printf ("fstat 5: ret = %d, errno = %d %s\n", ret, errno,
+ st.st_dev == 1 ? "OK" : "");
+ else
+ printf ("fstat 5: ret = %d, errno = %d\n", ret, errno);
+ stop ();
+ /* Standard error */
+ errno = 0;
+ ret = fstat (STDERR_FILENO, &st);
+ if (!ret)
+ printf ("fstat 6: ret = %d, errno = %d %s\n", ret, errno,
+ st.st_dev == 1 ? "OK" : "");
+ else
+ printf ("fstat 6: ret = %d, errno = %d\n", ret, errno);
+ stop ();
}
void
@@ -31,7 +31,7 @@ if {[is_remote host]} {
if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
executable \
- [list debug "additional_flags=-DOUTDIR=\"$outdir/\""]] != "" } {
+ [list debug "additional_flags=-DOUTDIR=\"$outdir/\" [target_info fileio,cflags]" "ldflags=[target_info fileio,ldflags]"]] != "" } {
untested "failed to compile"
return -1
}
@@ -136,6 +136,10 @@ gdb_test continue \
"Continuing\\..*close 2:.*EBADF$stop_msg" \
"Closing an invalid file descriptor returns EBADF"
+# Prepare some different file types for stat and fstat.
+set filetype_directory [file join $outdir "directory.fileio.test"]
+file mkdir $filetype_directory
+
gdb_test continue \
"Continuing\\..*stat 1:.*OK$stop_msg" \
"Stat a file"
@@ -152,6 +156,10 @@ gdb_test continue \
"Continuing\\..*stat 4:.*ENOENT$stop_msg" \
"Stat a nonexistant file returns ENOENT"
+gdb_test continue \
+"Continuing\\..*stat 5:.*OK$stop_msg" \
+"Stat a directory"
+
gdb_test continue \
"Continuing\\..*fstat 1:.*OK$stop_msg" \
"Fstat an open file"
@@ -160,6 +168,22 @@ gdb_test continue \
"Continuing\\..*fstat 2:.*EBADF$stop_msg" \
"Fstat an invalid file descriptor returns EBADF"
+gdb_test continue \
+"Continuing\\..*fstat 3:.*OK$stop_msg" \
+"Fstat a directory"
+
+gdb_test continue \
+"Continuing\\..*fstat 4:.*OK$stop_msg" \
+"Fstat standard output"
+
+gdb_test continue \
+"Continuing\\..*fstat 5:.*OK$stop_msg" \
+"Fstat standard input"
+
+gdb_test continue \
+"Continuing\\..*fstat 6:.*OK$stop_msg" \
+"Fstat standard error"
+
gdb_test continue \
"Continuing\\..*isatty 1:.*OK$stop_msg" \
"Isatty (stdin)"
@@ -1,3 +1,8 @@
+2018-07-05 Julio Guerra <julio@farjump.io>
+
+ * gdb/fileio.h: Add macro definitions for special files,
+ both for fst_dev and fst_mode fields of struct fst_stat.
+
2018-07-02 Maciej W. Rozycki <macro@mips.com>
PR tdep/8282
@@ -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_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 (FILEIO_S_IFIFO | FILEIO_S_IFCHR| \
+ FILEIO_S_IFDIR | FILEIO_S_IFBLK | FILEIO_S_IFREG | FILEIO_S_IFSOCK)
#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