public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] properly check error return from socket() and accept() calls
@ 2010-04-02  8:46 Ozkan Sezer
  2010-04-03  7:37 ` Ozkan Sezer
  0 siblings, 1 reply; 2+ messages in thread
From: Ozkan Sezer @ 2010-04-02  8:46 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

Hi:

socket() and accept() calls do not return "some negative value" on error,
they specifically return "-1". Therefore, the error return checks from
those calls must be done by checking equality to -1 not by being negative.
This is important for Win32 where the SOCKET type is actually unsigned
and the operating system has every right to return a value > INT_MAX and
< UINT_MAX and since gdb is using a signed int type for socket fd (which
is broken for Win64 where SOCKET type is uintptr_t, but that's for another
time), the negativity check may evaluate to true even if the socket() or
accept() call actually succeeded. The attached trivial patch fixes this as
described. For uniformity's sake, I also touched the files under sim/.
Please consider for applying. (The patch file has the ChangeLog. I don't
have write access.)

--
Ozkan

[-- Attachment #2: socket_err_checks.patch --]
[-- Type: application/octet-stream, Size: 7105 bytes --]

2010-04-02  Ozkan Sezer  <sezeroz@gmail.com>

gdb/
	* ser-tcp.c (net_open): Check error return from socket() call by its
	equality to -1 not by it being negative.

gdbserver/
	* gdbreplay.c (remote_open): Check error return from socket() call by
	its equality to -1 not by it being negative.
	* remote-utils.c (remote_open): Likewise.

sim/arm/
	* communicate.c (MYread_char): Check error return from accept() call
	by its equality to -1 not by it being negative.
	(MYread_charwait): Likewise.
	* main.c (main): Likewise for both socket() and accept() calls.

sim/common/
	* dv-sockser.c (dv_sockser_init): Check error return from socket()
	call by its equality to -1 not by it being negative.
	(connected_p): Likewise for accept() call.

sim/cris/
	* dv-rv.c (hw_rv_init_socket): Check error return from socket() call
	by its equality to -1 not by it being negative.
	* rvdummy.c (setupsocket): Likewise.
	(main): Likewise for accept() call as returned from setupsocket().

sim/m32c/
	* main.c (setup_tcp_console): Check error return from socket() call
	by its equality to -1 not by it being negative.

Index: gdb/ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.33
diff -u -p -r1.33 ser-tcp.c
--- gdb/ser-tcp.c	1 Jan 2010 07:31:41 -0000	1.33
+++ gdb/ser-tcp.c	2 Apr 2010 08:10:30 -0000
@@ -208,7 +208,7 @@ net_open (struct serial *scb, const char
   else
     scb->fd = socket (PF_INET, SOCK_STREAM, 0);
 
-  if (scb->fd < 0)
+  if (scb->fd == -1)
     return -1;
   
   /* set socket nonblocking */
Index: gdb/gdbserver/gdbreplay.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/gdbreplay.c,v
retrieving revision 1.24
diff -u -p -r1.24 gdbreplay.c
--- gdb/gdbserver/gdbreplay.c	1 Jan 2010 07:31:49 -0000	1.24
+++ gdb/gdbserver/gdbreplay.c	2 Apr 2010 08:10:31 -0000
@@ -210,7 +210,7 @@ remote_open (char *name)
 #endif
 
       tmp_desc = socket (PF_INET, SOCK_STREAM, 0);
-      if (tmp_desc < 0)
+      if (tmp_desc == -1)
 	perror_with_name ("Can't open socket");
 
       /* Allow rapid reuse of this port. */
Index: gdb/gdbserver/remote-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.71
diff -u -p -r1.71 remote-utils.c
--- gdb/gdbserver/remote-utils.c	20 Jan 2010 22:55:38 -0000	1.71
+++ gdb/gdbserver/remote-utils.c	2 Apr 2010 08:10:31 -0000
@@ -213,7 +213,7 @@ remote_open (char *name)
 #endif
 
       tmp_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
-      if (tmp_desc < 0)
+      if (tmp_desc == -1)
 	perror_with_name ("Can't open socket");
 
       /* Allow rapid reuse of this port. */

Index: sim/arm/communicate.c
===================================================================
RCS file: /cvs/src/src/sim/arm/communicate.c,v
retrieving revision 1.2
diff -u -p -r1.2 communicate.c
--- sim/arm/communicate.c	12 May 2005 07:36:59 -0000	1.2
+++ sim/arm/communicate.c	2 Apr 2010 08:10:34 -0000
@@ -83,7 +83,7 @@ retry:
 	  return -1;
 	  fprintf (stderr, "Waiting for connection from debugger...");
 	  debugsock = accept (sockethandle, &isa, &i);
-	  if (debugsock < 0)
+	  if (debugsock == -1)
 	    {			/* Now we are in serious trouble... */
 	      perror ("accept");
 	      return -1;
@@ -138,7 +138,7 @@ retry:
 	  return -1;
 	  fprintf (stderr, "Waiting for connection from debugger...");
 	  debugsock = accept (sockethandle, &isa, &i);
-	  if (debugsock < 0)
+	  if (debugsock == -1)
 	    {			/* Now we are in serious trouble... */
 	      perror ("accept");
 	      return -1;
Index: sim/arm/main.c
===================================================================
RCS file: /cvs/src/src/sim/arm/main.c,v
retrieving revision 1.2
diff -u -p -r1.2 main.c
--- sim/arm/main.c	12 May 2005 07:36:59 -0000	1.2
+++ sim/arm/main.c	2 Apr 2010 08:10:34 -0000
@@ -117,7 +117,7 @@ main (int argc, char *argv[])
 
   /* Open a socket */
   sockethandle = socket (hp->h_addrtype, SOCK_STREAM, 0);
-  if (sockethandle < 0)
+  if (sockethandle == -1)
     {
       perror ("socket");
       return 1;
@@ -147,7 +147,7 @@ main (int argc, char *argv[])
   fprintf (stderr, "Waiting for connection from debugger...");
 
   debugsock = accept (sockethandle, &isa, &i);
-  if (debugsock < 0)
+  if (debugsock == -1)
     {
       perror ("accept");
       return 1;
Index: sim/common/dv-sockser.c
===================================================================
RCS file: /cvs/src/src/sim/common/dv-sockser.c,v
retrieving revision 1.8
diff -u -p -r1.8 dv-sockser.c
--- sim/common/dv-sockser.c	30 Mar 2010 23:09:48 -0000	1.8
+++ sim/common/dv-sockser.c	2 Apr 2010 08:10:34 -0000
@@ -166,7 +166,7 @@ dv_sockser_init (SIM_DESC sd)
     }
 
   sockser_listen_fd = socket (PF_INET, SOCK_STREAM, 0);
-  if (sockser_listen_fd < 0)
+  if (sockser_listen_fd == -1)
     {
       sim_io_eprintf (sd, "sockser init: unable to get socket: %s\n",
 		      strerror (errno));
@@ -274,7 +274,7 @@ connected_p (SIM_DESC sd)
 
   addrlen = sizeof (sockaddr);
   sockser_fd = accept (sockser_listen_fd, &sockaddr, &addrlen);
-  if (sockser_fd < 0)
+  if (sockser_fd == -1)
     return 0;
 
   /* Set non-blocking i/o.  */
Index: sim/cris/dv-rv.c
===================================================================
RCS file: /cvs/src/src/sim/cris/dv-rv.c,v
retrieving revision 1.6
diff -u -p -r1.6 dv-rv.c
--- sim/cris/dv-rv.c	1 Jan 2010 10:03:28 -0000	1.6
+++ sim/cris/dv-rv.c	2 Apr 2010 08:10:34 -0000
@@ -887,7 +887,7 @@ hw_rv_init_socket (struct hw *me)
   server.sin_port = htons (rv->port);
   sock = socket (AF_INET, SOCK_STREAM, 0);
 
-  if (sock < 0)
+  if (sock == -1)
     hw_abort (me, "can't get a socket for %s:%d connection",
 	      rv->host, rv->port);
 
Index: sim/cris/rvdummy.c
===================================================================
RCS file: /cvs/src/src/sim/cris/rvdummy.c,v
retrieving revision 1.6
diff -u -p -r1.6 rvdummy.c
--- sim/cris/rvdummy.c	1 Jan 2010 10:03:28 -0000	1.6
+++ sim/cris/rvdummy.c	2 Apr 2010 08:10:34 -0000
@@ -118,7 +118,7 @@ int setupsocket (void)
   memset (&sa_in, 0, sizeof (sa_in));
 
   s = socket (AF_INET, SOCK_STREAM, 0);
-  if (s < 0)
+  if (s == -1)
     return -1;
 
   if (setsockopt (s, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof reuse) != 0)
@@ -517,7 +517,7 @@ main (int argc, char *argv[])
       }
 
   fd = setupsocket ();
-  if (fd < 0)
+  if (fd == -1)
     {
       fprintf (stderr, "%s: problem setting up the connection: %s\n",
 	       progname, strerror (errno));
Index: sim/m32c/main.c
===================================================================
RCS file: /cvs/src/src/sim/m32c/main.c,v
retrieving revision 1.9
diff -u -p -r1.9 main.c
--- sim/m32c/main.c	1 Jan 2010 10:03:31 -0000	1.9
+++ sim/m32c/main.c	2 Apr 2010 08:10:34 -0000
@@ -98,7 +98,7 @@ setup_tcp_console (char *portname)
   address.sin_port = htons (port);
 
   isocket = socket (AF_INET, SOCK_STREAM, 0);
-  if (isocket < 0)
+  if (isocket == -1)
     {
       perror ("socket");
       exit (1);

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] properly check error return from socket() and accept()   calls
  2010-04-02  8:46 [PATCH] properly check error return from socket() and accept() calls Ozkan Sezer
@ 2010-04-03  7:37 ` Ozkan Sezer
  0 siblings, 0 replies; 2+ messages in thread
From: Ozkan Sezer @ 2010-04-03  7:37 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

On Fri, Apr 2, 2010 at 11:46 AM, Ozkan Sezer <sezeroz@gmail.com> wrote:
> Hi:
>
> socket() and accept() calls do not return "some negative value" on error,
> they specifically return "-1". Therefore, the error return checks from
> those calls must be done by checking equality to -1 not by being negative.
> This is important for Win32 where the SOCKET type is actually unsigned
> and the operating system has every right to return a value > INT_MAX and
> < UINT_MAX and since gdb is using a signed int type for socket fd (which
> is broken for Win64 where SOCKET type is uintptr_t, but that's for another
> time), the negativity check may evaluate to true even if the socket() or
> accept() call actually succeeded. The attached trivial patch fixes this as
> described. For uniformity's sake, I also touched the files under sim/.
> Please consider for applying. (The patch file has the ChangeLog. I don't
> have write access.)
>

I seem to have missed a couple of places. Attached the updated patch.

--
Ozkan

[-- Attachment #2: gdb_socket_err_checks.patch --]
[-- Type: application/octet-stream, Size: 8144 bytes --]

2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>

gdb/
	* ser-tcp.c (net_open): Check error return from socket() call by its
	equality to -1 not by it being negative.
	(net_close): Likewise.

gdb/gdbserver/
	* gdbreplay.c (remote_open): Check error return from socket() call by
	its equality to -1 not by it being negative.
	* remote-utils.c (remote_open): Likewise.

sim/arm/
	* communicate.c (MYread_char): Check error return from accept() call
	by its equality to -1 not by it being negative.
	(MYread_charwait): Likewise.
	* main.c (main): Likewise for both socket() and accept() calls.

sim/common/
	* dv-sockser.c (dv_sockser_init): Check error return from socket()
	call by its equality to -1 not by it being negative.
	(connected_p): Likewise for accept() call.

sim/cris/
	* dv-rv.c (hw_rv_init_socket): Check error return from socket() call
	by its equality to -1 not by it being negative.
	(hw_rv_write): Likewise.
	(hw_rv_handle_incoming): Likewise.
	(hw_rv_poll_once): Likewise.
	* rvdummy.c (setupsocket): Likewise.
	(main): Likewise for accept() call as returned from setupsocket().

sim/m32c/
	* main.c (setup_tcp_console): Check error return from socket() call
	by its equality to -1 not by it being negative.

Index: gdb/ser-tcp.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-tcp.c,v
retrieving revision 1.33
diff -u -p -r1.33 ser-tcp.c
--- gdb/ser-tcp.c	1 Jan 2010 07:31:41 -0000	1.33
+++ gdb/ser-tcp.c	3 Apr 2010 07:30:23 -0000
@@ -208,7 +208,7 @@ net_open (struct serial *scb, const char
   else
     scb->fd = socket (PF_INET, SOCK_STREAM, 0);
 
-  if (scb->fd < 0)
+  if (scb->fd == -1)
     return -1;
   
   /* set socket nonblocking */
@@ -323,7 +323,7 @@ net_open (struct serial *scb, const char
 void
 net_close (struct serial *scb)
 {
-  if (scb->fd < 0)
+  if (scb->fd == -1)
     return;
 
   close (scb->fd);
Index: gdb/gdbserver/gdbreplay.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/gdbreplay.c,v
retrieving revision 1.24
diff -u -p -r1.24 gdbreplay.c
--- gdb/gdbserver/gdbreplay.c	1 Jan 2010 07:31:49 -0000	1.24
+++ gdb/gdbserver/gdbreplay.c	3 Apr 2010 07:30:24 -0000
@@ -210,7 +210,7 @@ remote_open (char *name)
 #endif
 
       tmp_desc = socket (PF_INET, SOCK_STREAM, 0);
-      if (tmp_desc < 0)
+      if (tmp_desc == -1)
 	perror_with_name ("Can't open socket");
 
       /* Allow rapid reuse of this port. */
Index: gdb/gdbserver/remote-utils.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.71
diff -u -p -r1.71 remote-utils.c
--- gdb/gdbserver/remote-utils.c	20 Jan 2010 22:55:38 -0000	1.71
+++ gdb/gdbserver/remote-utils.c	3 Apr 2010 07:30:24 -0000
@@ -213,7 +213,7 @@ remote_open (char *name)
 #endif
 
       tmp_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
-      if (tmp_desc < 0)
+      if (tmp_desc == -1)
 	perror_with_name ("Can't open socket");
 
       /* Allow rapid reuse of this port. */

Index: sim/arm/communicate.c
===================================================================
RCS file: /cvs/src/src/sim/arm/communicate.c,v
retrieving revision 1.2
diff -u -p -r1.2 communicate.c
--- sim/arm/communicate.c	12 May 2005 07:36:59 -0000	1.2
+++ sim/arm/communicate.c	3 Apr 2010 07:30:26 -0000
@@ -83,7 +83,7 @@ retry:
 	  return -1;
 	  fprintf (stderr, "Waiting for connection from debugger...");
 	  debugsock = accept (sockethandle, &isa, &i);
-	  if (debugsock < 0)
+	  if (debugsock == -1)
 	    {			/* Now we are in serious trouble... */
 	      perror ("accept");
 	      return -1;
@@ -138,7 +138,7 @@ retry:
 	  return -1;
 	  fprintf (stderr, "Waiting for connection from debugger...");
 	  debugsock = accept (sockethandle, &isa, &i);
-	  if (debugsock < 0)
+	  if (debugsock == -1)
 	    {			/* Now we are in serious trouble... */
 	      perror ("accept");
 	      return -1;
Index: sim/arm/main.c
===================================================================
RCS file: /cvs/src/src/sim/arm/main.c,v
retrieving revision 1.2
diff -u -p -r1.2 main.c
--- sim/arm/main.c	12 May 2005 07:36:59 -0000	1.2
+++ sim/arm/main.c	3 Apr 2010 07:30:26 -0000
@@ -117,7 +117,7 @@ main (int argc, char *argv[])
 
   /* Open a socket */
   sockethandle = socket (hp->h_addrtype, SOCK_STREAM, 0);
-  if (sockethandle < 0)
+  if (sockethandle == -1)
     {
       perror ("socket");
       return 1;
@@ -147,7 +147,7 @@ main (int argc, char *argv[])
   fprintf (stderr, "Waiting for connection from debugger...");
 
   debugsock = accept (sockethandle, &isa, &i);
-  if (debugsock < 0)
+  if (debugsock == -1)
     {
       perror ("accept");
       return 1;
Index: sim/common/dv-sockser.c
===================================================================
RCS file: /cvs/src/src/sim/common/dv-sockser.c,v
retrieving revision 1.8
diff -u -p -r1.8 dv-sockser.c
--- sim/common/dv-sockser.c	30 Mar 2010 23:09:48 -0000	1.8
+++ sim/common/dv-sockser.c	3 Apr 2010 07:30:26 -0000
@@ -166,7 +166,7 @@ dv_sockser_init (SIM_DESC sd)
     }
 
   sockser_listen_fd = socket (PF_INET, SOCK_STREAM, 0);
-  if (sockser_listen_fd < 0)
+  if (sockser_listen_fd == -1)
     {
       sim_io_eprintf (sd, "sockser init: unable to get socket: %s\n",
 		      strerror (errno));
@@ -274,7 +274,7 @@ connected_p (SIM_DESC sd)
 
   addrlen = sizeof (sockaddr);
   sockser_fd = accept (sockser_listen_fd, &sockaddr, &addrlen);
-  if (sockser_fd < 0)
+  if (sockser_fd == -1)
     return 0;
 
   /* Set non-blocking i/o.  */
Index: sim/cris/dv-rv.c
===================================================================
RCS file: /cvs/src/src/sim/cris/dv-rv.c,v
retrieving revision 1.6
diff -u -p -r1.6 dv-rv.c
--- sim/cris/dv-rv.c	1 Jan 2010 10:03:28 -0000	1.6
+++ sim/cris/dv-rv.c	3 Apr 2010 07:30:27 -0000
@@ -404,7 +404,7 @@ hw_rv_write (struct hw *me,
 
   /* If we don't have a valid fd here, it's because we got an error
      initially, and we suppressed that error.  */
-  if (rv->fd < 0)
+  if (rv->fd == -1)
     hw_abort (me, "couldn't open a connection to %s:%d because: %s",
 	      rv->host, rv->port, strerror (rv->saved_errno));
 
@@ -637,7 +637,7 @@ hw_rv_handle_incoming (struct hw *me,
     {
       hw_rv_read (me, cbuf, 3);
 
-      if (rv->fd < 0)
+      if (rv->fd == -1)
 	return;
 
       len = cbuf[0] + cbuf[1] * 256 - 3;
@@ -723,7 +723,7 @@ hw_rv_poll_once (struct hw *me)
   int ret;
   struct timeval tv;
 
-  if (rv->fd < 0)
+  if (rv->fd == -1)
     /* Connection has died or was never initiated.  */
     return;
 
@@ -887,7 +887,7 @@ hw_rv_init_socket (struct hw *me)
   server.sin_port = htons (rv->port);
   sock = socket (AF_INET, SOCK_STREAM, 0);
 
-  if (sock < 0)
+  if (sock == -1)
     hw_abort (me, "can't get a socket for %s:%d connection",
 	      rv->host, rv->port);
 
Index: sim/cris/rvdummy.c
===================================================================
RCS file: /cvs/src/src/sim/cris/rvdummy.c,v
retrieving revision 1.6
diff -u -p -r1.6 rvdummy.c
--- sim/cris/rvdummy.c	1 Jan 2010 10:03:28 -0000	1.6
+++ sim/cris/rvdummy.c	3 Apr 2010 07:30:27 -0000
@@ -118,7 +118,7 @@ int setupsocket (void)
   memset (&sa_in, 0, sizeof (sa_in));
 
   s = socket (AF_INET, SOCK_STREAM, 0);
-  if (s < 0)
+  if (s == -1)
     return -1;
 
   if (setsockopt (s, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof reuse) != 0)
@@ -517,7 +517,7 @@ main (int argc, char *argv[])
       }
 
   fd = setupsocket ();
-  if (fd < 0)
+  if (fd == -1)
     {
       fprintf (stderr, "%s: problem setting up the connection: %s\n",
 	       progname, strerror (errno));
Index: sim/m32c/main.c
===================================================================
RCS file: /cvs/src/src/sim/m32c/main.c,v
retrieving revision 1.9
diff -u -p -r1.9 main.c
--- sim/m32c/main.c	1 Jan 2010 10:03:31 -0000	1.9
+++ sim/m32c/main.c	3 Apr 2010 07:30:27 -0000
@@ -98,7 +98,7 @@ setup_tcp_console (char *portname)
   address.sin_port = htons (port);
 
   isocket = socket (AF_INET, SOCK_STREAM, 0);
-  if (isocket < 0)
+  if (isocket == -1)
     {
       perror ("socket");
       exit (1);

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-04-03  7:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-02  8:46 [PATCH] properly check error return from socket() and accept() calls Ozkan Sezer
2010-04-03  7:37 ` Ozkan Sezer

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).