Remove the restriction of File I/O function open() to regular files only

Message ID 010201636b2573af-0cdb68f4-b1a1-4bc4-89b4-ba2e907f5524-000000@eu-west-1.amazonses.com
State New, archived
Headers

Commit Message

Julio Guerra May 16, 2018, 10:50 p.m. UTC
  The major goal is being able to write advanced embedded testing functions, like:
- reading/writing to a dedicated fifo between the embedded program and the host,
  instead of being restricted to the GDB console only.
- mocking features based on host's (e.g. opening /dev/random).

Tested with https://github.com/farjump/raspberry-pi/blob/ea31c48d7c7eed27d39fb1bec2d3a1d308cb8ae7/src/semihosting/semihosting.c

Also answers to an old RFC I did on the ML:
https://sourceware.org/ml/gdb/2016-07/msg00003.html

2018-05-16  Julio Guerra  <julio@farjump.io>

	* remote-fileio.c: allow using File I/O function open() with special
	files.

Signed-off-by: Julio Guerra <julio@farjump.io>
---
 gdb/ChangeLog       |  5 +++++
 gdb/remote-fileio.c | 24 ------------------------
 2 files changed, 5 insertions(+), 24 deletions(-)

-- 
2.17.0
  

Comments

Pedro Alves May 17, 2018, 1:32 p.m. UTC | #1
On 05/16/2018 11:50 PM, Julio Guerra wrote:
> The major goal is being able to write advanced embedded testing functions, like:
> - reading/writing to a dedicated fifo between the embedded program and the host,
>   instead of being restricted to the GDB console only.
> - mocking features based on host's (e.g. opening /dev/random).
> 
> Tested with https://github.com/farjump/raspberry-pi/blob/ea31c48d7c7eed27d39fb1bec2d3a1d308cb8ae7/src/semihosting/semihosting.c
> 
> Also answers to an old RFC I did on the ML:
> https://sourceware.org/ml/gdb/2016-07/msg00003.html
> 
> 2018-05-16  Julio Guerra  <julio@farjump.io>
> 
> 	* remote-fileio.c: allow using File I/O function open() with special
> 	files.

Does this affect gdb.base/fileio.exp?  For example, there we have:

 gdb_test continue \
 "Continuing\\..*open 3:.*EISDIR$stop_msg" \
 "Open directory for writing returns EISDIR"

And you seem to be removing code that makes that happen.

Would it be possible to portably update that test to exercise
the new possibilities the patch is proposing?

Thanks,
Pedro Alves

> 
> Signed-off-by: Julio Guerra <julio@farjump.io>
> ---
>  gdb/ChangeLog       |  5 +++++
>  gdb/remote-fileio.c | 24 ------------------------
>  2 files changed, 5 insertions(+), 24 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 57a4075a12..7217be67b6 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-05-16  Julio Guerra  <julio@farjump.io>
> +
> +	* remote-fileio.c: allow using File I/O function open() with special
> +	files.
> +
>  2018-05-15  Tamar Christina  <tamar.christina@arm.com>
>  
>  	PR binutils/21446
> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
> index 4c648a9c83..fa3cb15033 100644
> --- a/gdb/remote-fileio.c
> +++ b/gdb/remote-fileio.c
> @@ -407,24 +407,6 @@ remote_fileio_func_open (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 (-1, FILEIO_ENODEV);
> -	  return;
> -	}
> -      if (S_ISDIR (st.st_mode)
> -	  && ((flags & O_WRONLY) == O_WRONLY || (flags & O_RDWR) == O_RDWR))
> -	{
> -	  remote_fileio_reply (-1, FILEIO_EISDIR);
> -	  return;
> -	}
> -    }
> -
>    fd = gdb_open_cloexec (pathname, flags, mode);
>    if (fd < 0)
>      {
> @@ -885,12 +867,6 @@ remote_fileio_func_stat (char *buf)
>        remote_fileio_return_errno (-1);
>        return;
>      }
> -  /* Only operate on regular files and directories.  */
> -  if (!ret && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode))
> -    {
> -      remote_fileio_reply (-1, FILEIO_EACCES);
> -      return;
> -    }
>    if (statptr)
>      {
>        host_to_fileio_stat (&st, &fst);
>
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 57a4075a12..7217be67b6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-05-16  Julio Guerra  <julio@farjump.io>
+
+	* remote-fileio.c: allow using File I/O function open() with special
+	files.
+
 2018-05-15  Tamar Christina  <tamar.christina@arm.com>
 
 	PR binutils/21446
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index 4c648a9c83..fa3cb15033 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -407,24 +407,6 @@  remote_fileio_func_open (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 (-1, FILEIO_ENODEV);
-	  return;
-	}
-      if (S_ISDIR (st.st_mode)
-	  && ((flags & O_WRONLY) == O_WRONLY || (flags & O_RDWR) == O_RDWR))
-	{
-	  remote_fileio_reply (-1, FILEIO_EISDIR);
-	  return;
-	}
-    }
-
   fd = gdb_open_cloexec (pathname, flags, mode);
   if (fd < 0)
     {
@@ -885,12 +867,6 @@  remote_fileio_func_stat (char *buf)
       remote_fileio_return_errno (-1);
       return;
     }
-  /* Only operate on regular files and directories.  */
-  if (!ret && !S_ISREG (st.st_mode) && !S_ISDIR (st.st_mode))
-    {
-      remote_fileio_reply (-1, FILEIO_EACCES);
-      return;
-    }
   if (statptr)
     {
       host_to_fileio_stat (&st, &fst);