[2/2] sim: use gnulib to set nonblocking mode

Message ID 20210909063836.16875-2-vapier@gentoo.org
State New
Headers show
Series
  • [1/2] gnulib: import nonblocking module
Related show

Commit Message

Tom Tromey via Gdb-patches Sept. 9, 2021, 6:38 a.m.
Replace various custom ad-hoc fcntl/O_NONBLOCK implementations with
the gnulib nonblocking module.  This makes our code much tidier and
portable to other systems (e.g. Windows).
---
 sim/bfin/dv-bfin_emac.c |  5 +++--
 sim/common/dv-sockser.c | 11 +++--------
 sim/common/sim-io.c     | 22 +++++++---------------
 sim/ppc/main.c          | 20 ++++++--------------
 4 files changed, 19 insertions(+), 39 deletions(-)

-- 
2.33.0

Comments

Tom Tromey via Gdb-patches Sept. 9, 2021, 7:24 a.m. | #1
> Date: Thu,  9 Sep 2021 02:38:36 -0400

> From: Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>

> 

> Replace various custom ad-hoc fcntl/O_NONBLOCK implementations with

> the gnulib nonblocking module.  This makes our code much tidier and

> portable to other systems (e.g. Windows).


I'm not familiar with the sim's code, but Gnulib AFAIK supports
non-blocking flag on MinGW only for named pipes and sockets, not for
real files.  Are all the file descriptors in the places where you make
the changes of one of those kinds?  E.g., I see the fcntl calls on
file descriptor zero in some places.
Tom Tromey via Gdb-patches Sept. 9, 2021, 7:33 a.m. | #2
On 09 Sep 2021 10:24, Eli Zaretskii wrote:
> > Date: Thu,  9 Sep 2021 02:38:36 -0400

> > From: Mike Frysinger via Gdb-patches <gdb-patches@sourceware.org>

> > 

> > Replace various custom ad-hoc fcntl/O_NONBLOCK implementations with

> > the gnulib nonblocking module.  This makes our code much tidier and

> > portable to other systems (e.g. Windows).

> 

> I'm not familiar with the sim's code, but Gnulib AFAIK supports

> non-blocking flag on MinGW only for named pipes and sockets, not for

> real files.  Are all the file descriptors in the places where you make

> the changes of one of those kinds?  E.g., I see the fcntl calls on

> file descriptor zero in some places.


sim only uses nonblocking on sockets or tty's (e.g. stdin), not files on disk.
i don't know offhand about the specifics of Windows stdio and whether those are
actually pipes.
-mike
Tom Tromey via Gdb-patches Sept. 9, 2021, 7:44 a.m. | #3
> Date: Thu, 9 Sep 2021 03:33:23 -0400

> From: Mike Frysinger <vapier@gentoo.org>

> Cc: gdb-patches@sourceware.org

> 

> > I'm not familiar with the sim's code, but Gnulib AFAIK supports

> > non-blocking flag on MinGW only for named pipes and sockets, not for

> > real files.  Are all the file descriptors in the places where you make

> > the changes of one of those kinds?  E.g., I see the fcntl calls on

> > file descriptor zero in some places.

> 

> sim only uses nonblocking on sockets or tty's (e.g. stdin), not files on disk.

> i don't know offhand about the specifics of Windows stdio and whether those are

> actually pipes.


I don't think Gnulib's set_nonblocking_flag works on descriptors for
the console devices, so I think the code should be prepared to see
ENOSYS and ENOTSUP in errno in those cases.

Patch

diff --git a/sim/bfin/dv-bfin_emac.c b/sim/bfin/dv-bfin_emac.c
index e5ceedbef514..b53191d3838a 100644
--- a/sim/bfin/dv-bfin_emac.c
+++ b/sim/bfin/dv-bfin_emac.c
@@ -35,6 +35,8 @@ 
 #include <linux/if_tun.h>
 #endif
 
+#include "nonblocking.h"
+
 #ifdef HAVE_LINUX_IF_TUN_H
 # define WITH_TUN 1
 #else
@@ -570,8 +572,7 @@  bfin_emac_tap_init (struct hw *me)
       return;
     }
 
-  flags = fcntl (emac->tap, F_GETFL);
-  fcntl (emac->tap, F_SETFL, flags | O_NONBLOCK);
+  set_nonblocking_flag (emac->tap, true);
 #endif
 }
 
diff --git a/sim/common/dv-sockser.c b/sim/common/dv-sockser.c
index 477e8f681af7..5423765c66a3 100644
--- a/sim/common/dv-sockser.c
+++ b/sim/common/dv-sockser.c
@@ -24,9 +24,6 @@  along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <string.h>
 #include <signal.h>
 #include <stdlib.h>
-#ifdef HAVE_FCNTL_H
-#include <fcntl.h>
-#endif
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
 #endif
@@ -40,6 +37,8 @@  along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <sys/select.h>
 #include <sys/socket.h>
 
+#include "nonblocking.h"
+
 #include "sim-main.h"
 #include "sim-assert.h"
 #include "sim-options.h"
@@ -256,17 +255,13 @@  connected_p (SIM_DESC sd)
     return 0;
 
   /* Set non-blocking i/o.  */
-#if defined(F_GETFL) && defined(O_NONBLOCK)
-  flags = fcntl (sockser_fd, F_GETFL);
-  flags |= O_NONBLOCK;
-  if (fcntl (sockser_fd, F_SETFL, flags) == -1)
+  if (set_nonblocking_flag (sockser_fd, true))
     {
       sim_io_eprintf (sd, "unable to set nonblocking i/o");
       close (sockser_fd);
       sockser_fd = -1;
       return 0;
     }
-#endif
   return 1;
 }
 
diff --git a/sim/common/sim-io.c b/sim/common/sim-io.c
index a5a7ff17b067..9398cba57233 100644
--- a/sim/common/sim-io.c
+++ b/sim/common/sim-io.c
@@ -22,15 +22,14 @@ 
 /* This must come before any other includes.  */
 #include "defs.h"
 
+#include "nonblocking.h"
+
 #include "sim-main.h"
 #include "sim-io.h"
 #include "sim/callback.h"
 #include "targ-vals.h"
 
 #include <errno.h>
-#if HAVE_FCNTL_H
-#include <fcntl.h>
-#endif
 
 #if HAVE_UNISTD_H
 #include <unistd.h>
@@ -350,23 +349,20 @@  sim_io_poll_read (SIM_DESC sd,
 		  char *buf,
 		  int sizeof_buf)
 {
-#if defined(O_NONBLOCK) && defined(F_GETFL) && defined(F_SETFL)
   int fd = STATE_CALLBACK (sd)->fdmap[sim_io_fd];
-  int flags;
   int status;
   int nr_read;
   int result;
   STATE_CALLBACK (sd)->last_errno = 0;
   /* get the old status */
-  flags = fcntl (fd, F_GETFL, 0);
-  if (flags == -1)
+  status = get_nonblocking_flag (fd);
+  if (status == -1)
     {
-      perror ("sim_io_poll_read");
+      perror ("sim_io_read_stdin");
       return 0;
     }
   /* temp, disable blocking IO */
-  status = fcntl (fd, F_SETFL, flags | O_NONBLOCK);
-  if (status == -1)
+  if (status == 0 && set_nonblocking_flag (fd, true) == -1)
     {
       perror ("sim_io_read_stdin");
       return 0;
@@ -384,16 +380,12 @@  sim_io_poll_read (SIM_DESC sd,
       STATE_CALLBACK (sd)->last_errno = errno;
     }
   /* return to regular vewing */
-  status = fcntl (fd, F_SETFL, flags);
-  if (status == -1)
+  if (status == 0 && set_nonblocking_flag (fd, false) == -1)
     {
       perror ("sim_io_read_stdin");
       /* return 0; */
     }
   return result;
-#else
-  return sim_io_read (sd, sim_io_fd, buf, sizeof_buf);
-#endif
 }
 
 int
diff --git a/sim/ppc/main.c b/sim/ppc/main.c
index d9a40700973d..f64b163afea7 100644
--- a/sim/ppc/main.c
+++ b/sim/ppc/main.c
@@ -26,6 +26,8 @@ 
 
 #include <signal.h>
 
+#include "nonblocking.h"
+
 #include "psim.h"
 #include "options.h"
 #include "device.h" /* FIXME: psim should provide the interface */
@@ -42,11 +44,6 @@ 
 #include <string.h>
 #include <errno.h>
 
-#if !defined(O_NONBLOCK) || !defined(F_GETFL) || !defined(F_SETFL)
-#undef WITH_STDIO
-#define WITH_STDIO DO_USE_STDIO
-#endif
-
 
 extern char **environ;
 
@@ -152,22 +149,19 @@  sim_io_read_stdin(char *buf,
     return sim_io_eof;
     break;
   case DONT_USE_STDIO:
-#if defined(O_NONBLOCK) && defined(F_GETFL) && defined(F_SETFL)
     {
       /* check for input */
-      int flags;
       int status;
       int nr_read;
       int result;
       /* get the old status */
-      flags = fcntl(0, F_GETFL, 0);
-      if (flags == -1) {
+      status = get_nonblocking_flag (0);
+      if (status == -1) {
 	perror("sim_io_read_stdin");
 	return sim_io_eof;
       }
       /* temp, disable blocking IO */
-      status = fcntl(0, F_SETFL, flags | O_NONBLOCK);
-      if (status == -1) {
+      if (status == 0 && set_nonblocking_flag (0, true) == -1) {
 	perror("sim_io_read_stdin");
 	return sim_io_eof;
       }
@@ -185,15 +179,13 @@  sim_io_read_stdin(char *buf,
 	  result = sim_io_eof;
       }
       /* return to regular vewing */
-      status = fcntl(0, F_SETFL, flags);
-      if (status == -1) {
+      if (status == 0 && set_nonblocking_flag (0, false) == -1) {
 	perror("sim_io_read_stdin");
 	return sim_io_eof;
       }
       return result;
     }
     break;
-#endif
   default:
     error("sim_io_read_stdin: invalid switch\n");
     break;