PING: [PATCH] properly check error return from socket() and accept() calls

PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

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

PING.

On Sat, Apr 3, 2010 at 10:37 AM, Ozkan Sezer <sezeroz@gmail.com> wrote:
> 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);

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Tom Tromey]

>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:

Ozkan> PING.

Ozkan> 2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>
Ozkan> gdb/
Ozkan> 	* ser-tcp.c (net_open): Check error return from socket() call by its
Ozkan> 	equality to -1 not by it being negative.
Ozkan> 	(net_close): Likewise.
[...]

It seems reasonable enough to me.

Do you have a copyright assignment in place?  TBH it isn't clear to me
whether this patch requires one -- it is long enough, but I would say
that there is really only one way to write this patch.

Tom

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

On Fri, Apr 9, 2010 at 9:45 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>
> Ozkan> PING.
>
> Ozkan> 2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>
> Ozkan> gdb/
> Ozkan>  * ser-tcp.c (net_open): Check error return from socket() call by its
> Ozkan>  equality to -1 not by it being negative.
> Ozkan>  (net_close): Likewise.
> [...]
>
> It seems reasonable enough to me.
>

Thanks.

> Do you have a copyright assignment in place?  TBH it isn't clear to me
> whether this patch requires one -- it is long enough, but I would say
> that there is really only one way to write this patch.
>
> Tom
>

I have for binutils, I can't remember whether I do for gdb.
If it really is required, please tell me that I contact FSF
for it.

--
Ozkan

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

On Fri, Apr 9, 2010 at 9:47 PM, Ozkan Sezer <sezeroz@gmail.com> wrote:
> On Fri, Apr 9, 2010 at 9:45 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>>
>> Ozkan> PING.
>>
>> Ozkan> 2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>
>> Ozkan> gdb/
>> Ozkan>  * ser-tcp.c (net_open): Check error return from socket() call by its
>> Ozkan>  equality to -1 not by it being negative.
>> Ozkan>  (net_close): Likewise.
>> [...]
>>
>> It seems reasonable enough to me.
>>
>
> Thanks.
>
>> Do you have a copyright assignment in place?  TBH it isn't clear to me
>> whether this patch requires one -- it is long enough, but I would say
>> that there is really only one way to write this patch.
>>
>> Tom
>>
>
> I have for binutils, I can't remember whether I do for gdb.
> If it really is required, please tell me that I contact FSF
> for it.
>
> --
> Ozkan
>

FWIW, I just wrote to assign@gnu.org for a copyright
assignment for gdb, although I assume it may take time
for them to get back to me, especially when it comes
to the snail mail phase.

Regards.

--
Ozkan

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

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

On Fri, Apr 9, 2010 at 9:45 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>
> Ozkan> PING.
>
> Ozkan> 2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>
> Ozkan> gdb/
> Ozkan>  * ser-tcp.c (net_open): Check error return from socket() call by its
> Ozkan>  equality to -1 not by it being negative.
> Ozkan>  (net_close): Likewise.
> [...]
>
> It seems reasonable enough to me.
>
> Do you have a copyright assignment in place?  TBH it isn't clear to me
> whether this patch requires one -- it is long enough, but I would say
> that there is really only one way to write this patch.
>
> Tom
>

I started the copyright assignment, 564768 is the number
they gave to me, but it has been almost a month and they
aren't responding to my emails, either.  So I don't know
what is going on with the paper work.

I am attaching an up-to-date version of the patch which
applies cleanly to the current cvs.

Regards.

--
Ozkan

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

2010-05-15  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.76
diff -u -p -r1.76 remote-utils.c
--- gdb/gdbserver/remote-utils.c	3 May 2010 20:53:21 -0000	1.76
+++ gdb/gdbserver/remote-utils.c	5 May 2010 06:28:15 -0000
@@ -305,7 +305,7 @@ remote_open (char *name)
 #endif
 
       listen_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
-      if (listen_desc < 0)
+      if (listen_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);

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

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

On Fri, Apr 9, 2010 at 9:45 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>
> Ozkan> PING.
>
> Ozkan> 2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>
> Ozkan> gdb/
> Ozkan>  * ser-tcp.c (net_open): Check error return from socket() call by its
> Ozkan>  equality to -1 not by it being negative.
> Ozkan>  (net_close): Likewise.
> [...]
>
> It seems reasonable enough to me.
>
> Do you have a copyright assignment in place?  TBH it isn't clear to me
> whether this patch requires one -- it is long enough, but I would say
> that there is really only one way to write this patch.
>
> Tom
>

Hello: My GDB assignment/disclaimer process with the FSF is currently
complete (RT 564768). A current, cleanly applying version of the patch
is attached. If someone could please apply it for me, I'd be grateful.
Thanks.

--
Ozkan

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

2010-05-26  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.34
diff -u -p -r1.34 ser-tcp.c
--- gdb/ser-tcp.c	16 May 2010 23:49:58 -0000	1.34
+++ gdb/ser-tcp.c	23 May 2010 11:02:02 -0000
@@ -209,7 +209,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 */
@@ -325,7 +325,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.76
diff -u -p -r1.76 remote-utils.c
--- gdb/gdbserver/remote-utils.c	3 May 2010 20:53:21 -0000	1.76
+++ gdb/gdbserver/remote-utils.c	5 May 2010 06:28:15 -0000
@@ -305,7 +305,7 @@ remote_open (char *name)
 #endif
 
       listen_desc = socket (PF_INET, SOCK_STREAM, IPPROTO_TCP);
-      if (listen_desc < 0)
+      if (listen_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);

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

On Fri, Apr 9, 2010 at 9:45 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>
> Ozkan> PING.
>
> Ozkan> 2010-04-03  Ozkan Sezer  <sezeroz@gmail.com>
> Ozkan> gdb/
> Ozkan>  * ser-tcp.c (net_open): Check error return from socket() call by its
> Ozkan>  equality to -1 not by it being negative.
> Ozkan>  (net_close): Likewise.
> [...]
>
> It seems reasonable enough to me.
>
> Do you have a copyright assignment in place?  TBH it isn't clear to me
> whether this patch requires one -- it is long enough, but I would say
> that there is really only one way to write this patch.
>
> Tom
>

Committed now. Thanks.

--
Ozkan

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Tom Tromey]

>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:

Ozkan> Committed now. Thanks.

Thanks.  Please also send and commit a patch to add yourself to the
MAINTAINERS file, in the write-after-approval section.

Tom

Re: PING: [PATCH] properly check error return from socket() and accept() calls

[Ozkan Sezer]

On Thu, May 27, 2010 at 6:42 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Ozkan" == Ozkan Sezer <sezeroz@gmail.com> writes:
>
> Ozkan> Committed now. Thanks.
>
> Thanks.  Please also send and commit a patch to add yourself to the
> MAINTAINERS file, in the write-after-approval section.
>
> Tom
>

Done. Thanks.

--
Ozkan