[PATCH] sim: use socklen_t with accept()

[PATCH] sim: use socklen_t with accept()

[Mike Frysinger]

The accept() function takes a socklen_t, not an int.  Using an int causes:
dv-sockser.c: In function 'connected_p':
dv-sockser.c:273: warning: pointer targets in passing argument 3
                           of 'accept' differ in signedness

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
2010-03-29  Mike Frysinger  <vapier@gentoo.org>

	* dv-sockser.c (connected_p): Declare addrlen as socklen_t.

 sim/common/dv-sockser.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sim/common/dv-sockser.c b/sim/common/dv-sockser.c
index 9195190..c584fc7 100644
--- a/sim/common/dv-sockser.c
+++ b/sim/common/dv-sockser.c
@@ -243,7 +243,7 @@ connected_p (SIM_DESC sd)
   struct timeval tv;
   fd_set readfds;
   struct sockaddr sockaddr;
-  int addrlen;
+  socklen_t addrlen;
 
   if (sockser_listen_fd == -1)
     return 0;
-- 
1.7.0.2

Re: [PATCH] sim: use socklen_t with accept()

[Tom Tromey]

>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

Mike> The accept() function takes a socklen_t, not an int.

I thought there were still platforms where accept took an int*.

gnulib at least implies it:

http://www.gnu.org/software/gnulib/manual/html_node/accept.html

... but doesn't mention which ones.

Given that both gdb and gdbserver check for socklen_t and typedef it if
it does not exist, I think sim ought to as well.

Tom

Re: [PATCH] sim: use socklen_t with accept()

[Andreas Schwab]

Tom Tromey <tromey@redhat.com> writes:

>>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:
>
> Mike> The accept() function takes a socklen_t, not an int.
>
> I thought there were still platforms where accept took an int*.

Those platforms either define socklen_t to int (which is actually the
de-facto standard), or not at all.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[PATCH v2] sim: use socklen_t with accept()

[Mike Frysinger]

The accept() function takes a socklen_t, not an int.  Using an int causes:
dv-sockser.c: In function 'connected_p':
dv-sockser.c:273: warning: pointer targets in passing argument 3
                           of 'accept' differ in signedness

So use the same socklen_t detection code as gdb and convert the accept().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v2
	- pull in socklen_t configure checks as suggested by people

2010-03-30  Mike Frysinger  <vapier@gentoo.org>

	* configure.ac: Check for socklen_t.
	* configure, config.in: Regenerated.
	* dv-sockser.c (connected_p): Change addrlen type to socklen_t.

Note: this config.in diffstat is larger because the regen tweaked the
      comment style for a lot of defines ... not sure if this is correct
 sim/common/config.in    |   29 +++++++++++---------
 sim/common/configure    |   68 +++++++++++++++++++++++++++++++++++++++++++++++
 sim/common/configure.ac |    5 +++
 sim/common/dv-sockser.c |    2 +-
 4 files changed, 90 insertions(+), 14 deletions(-)

diff --git a/sim/common/configure.ac b/sim/common/configure.ac
index 5f5845a..39302c8 100644
--- a/sim/common/configure.ac
+++ b/sim/common/configure.ac
@@ -49,5 +49,10 @@ AC_CHECK_MEMBERS([[struct stat.st_dev], [struct stat.st_ino],
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif]])
+AC_CHECK_TYPES(socklen_t, [], [],
+[#include <sys/types.h>
+#include <sys/socket.h>
+])
+
 AC_OUTPUT(Makefile,
 [case x$CONFIG_HEADERS in xcconfig.h:config.in) echo > stamp-h ;; esac])
diff --git a/sim/common/dv-sockser.c b/sim/common/dv-sockser.c
index 9195190..c584fc7 100644
--- a/sim/common/dv-sockser.c
+++ b/sim/common/dv-sockser.c
@@ -243,7 +243,7 @@ connected_p (SIM_DESC sd)
   struct timeval tv;
   fd_set readfds;
   struct sockaddr sockaddr;
-  int addrlen;
+  socklen_t addrlen;
 
   if (sockser_listen_fd == -1)
     return 0;
-- 
1.7.0.2

Re: [PATCH v2] sim: use socklen_t with accept()

[Tom Tromey]

>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

Mike> 2010-03-30  Mike Frysinger  <vapier@gentoo.org>
Mike> 	* configure.ac: Check for socklen_t.
Mike> 	* configure, config.in: Regenerated.
Mike> 	* dv-sockser.c (connected_p): Change addrlen type to socklen_t.

Somewhere, probably dv-sockser.c, you also need this bit:

#ifndef HAVE_SOCKLEN_T
typedef int socklen_t;
#endif

Mike> Note: this config.in diffstat is larger because the regen tweaked the
Mike>       comment style for a lot of defines ... not sure if this is correct

Make sure you use the exact same versions of the autotools as currently
used in the tree.  I download the canonical versions from gnu.org and
install them all in a fresh $prefix for gdb hacking.  You can't use
different versions (even minor versions) or distro variants, as they
cause churn in the repository.

Tom

[PATCH v3] sim: use socklen_t with accept()

[Mike Frysinger]

The accept() function takes a socklen_t, not an int.  Using an int causes:
dv-sockser.c: In function 'connected_p':
dv-sockser.c:273: warning: pointer targets in passing argument 3
                           of 'accept' differ in signedness

So use the same socklen_t detection code as gdb and convert the accept().

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
v3
	- have dv-sockser.c declare socklen_t fallback

2010-03-30  Mike Frysinger  <vapier@gentoo.org>

	* configure.ac: Check for socklen_t.
	* configure, config.in: Regenerated.
	* dv-sockser.c: Typedef socklen_t as int when !HAVE_SOCKLEN_T.
	(connected_p): Change addrlen type to socklen_t.

 sim/common/config.in    |   29 +++++++++++---------
 sim/common/configure    |   68 +++++++++++++++++++++++++++++++++++++++++++++++
 sim/common/configure.ac |    5 +++
 sim/common/dv-sockser.c |    6 +++-
 4 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/sim/common/config.in b/sim/common/config.in
index 6776827..6f93ff4 100644
--- a/sim/common/config.in
+++ b/sim/common/config.in
@@ -46,6 +46,9 @@
 /* Define to 1 if you have the `sigaction' function. */
 #undef HAVE_SIGACTION
 
+/* Define to 1 if the system has the type `socklen_t'. */
+#undef HAVE_SOCKLEN_T
+
 /* Define to 1 if you have the <stdint.h> header file. */
 #undef HAVE_STDINT_H
 
@@ -58,43 +61,43 @@
 /* Define to 1 if you have the <string.h> header file. */
 #undef HAVE_STRING_H
 
-/* Define to 1 if st_atime is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_atime'. */
 #undef HAVE_STRUCT_STAT_ST_ATIME
 
-/* Define to 1 if st_blksize is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_blksize'. */
 #undef HAVE_STRUCT_STAT_ST_BLKSIZE
 
-/* Define to 1 if st_blocks is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_blocks'. */
 #undef HAVE_STRUCT_STAT_ST_BLOCKS
 
-/* Define to 1 if st_ctime is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_ctime'. */
 #undef HAVE_STRUCT_STAT_ST_CTIME
 
-/* Define to 1 if st_dev is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_dev'. */
 #undef HAVE_STRUCT_STAT_ST_DEV
 
-/* Define to 1 if st_gid is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_gid'. */
 #undef HAVE_STRUCT_STAT_ST_GID
 
-/* Define to 1 if st_ino is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_ino'. */
 #undef HAVE_STRUCT_STAT_ST_INO
 
-/* Define to 1 if st_mode is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_mode'. */
 #undef HAVE_STRUCT_STAT_ST_MODE
 
-/* Define to 1 if st_mtime is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_mtime'. */
 #undef HAVE_STRUCT_STAT_ST_MTIME
 
-/* Define to 1 if st_nlink is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_nlink'. */
 #undef HAVE_STRUCT_STAT_ST_NLINK
 
-/* Define to 1 if st_rdev is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_rdev'. */
 #undef HAVE_STRUCT_STAT_ST_RDEV
 
-/* Define to 1 if st_size is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_size'. */
 #undef HAVE_STRUCT_STAT_ST_SIZE
 
-/* Define to 1 if st_uid is a member of struct stat. */
+/* Define to 1 if `struct stat' is a member of `st_uid'. */
 #undef HAVE_STRUCT_STAT_ST_UID
 
 /* Define to 1 if you have the <sys/mman.h> header file. */
diff --git a/sim/common/configure.ac b/sim/common/configure.ac
index 5f5845a..39302c8 100644
--- a/sim/common/configure.ac
+++ b/sim/common/configure.ac
@@ -49,5 +49,10 @@ AC_CHECK_MEMBERS([[struct stat.st_dev], [struct stat.st_ino],
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif]])
+AC_CHECK_TYPES(socklen_t, [], [],
+[#include <sys/types.h>
+#include <sys/socket.h>
+])
+
 AC_OUTPUT(Makefile,
 [case x$CONFIG_HEADERS in xcconfig.h:config.in) echo > stamp-h ;; esac])
diff --git a/sim/common/dv-sockser.c b/sim/common/dv-sockser.c
index 9195190..03d49da 100644
--- a/sim/common/dv-sockser.c
+++ b/sim/common/dv-sockser.c
@@ -55,6 +55,10 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "dv-sockser.h"
 \f
+#ifndef HAVE_SOCKLEN_T
+typedef int socklen_t;
+#endif
+
 /* Get definitions for both O_NONBLOCK and O_NDELAY.  */
 
 #ifndef O_NDELAY
@@ -243,7 +247,7 @@ connected_p (SIM_DESC sd)
   struct timeval tv;
   fd_set readfds;
   struct sockaddr sockaddr;
-  int addrlen;
+  socklen_t addrlen;
 
   if (sockser_listen_fd == -1)
     return 0;
-- 
1.7.0.2

Re: [PATCH v2] sim: use socklen_t with accept()

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1282 bytes --]

On Tuesday 30 March 2010 18:21:55 Tom Tromey wrote:
> >>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:
> Mike> 2010-03-30  Mike Frysinger  <vapier@gentoo.org>
> Mike> 	* configure.ac: Check for socklen_t.
> Mike> 	* configure, config.in: Regenerated.
> Mike> 	* dv-sockser.c (connected_p): Change addrlen type to socklen_t.
> 
> Somewhere, probably dv-sockser.c, you also need this bit:
> 
> #ifndef HAVE_SOCKLEN_T
> typedef int socklen_t;
> #endif

i would have thought the configure code people were using would take care of 
this, but oh well.

> Mike> Note: this config.in diffstat is larger because the regen tweaked the
> Mike>       comment style for a lot of defines ... not sure if this is
> correct
> 
> Make sure you use the exact same versions of the autotools as currently
> used in the tree.  I download the canonical versions from gnu.org and
> install them all in a fresh $prefix for gdb hacking.  You can't use
> different versions (even minor versions) or distro variants, as they
> cause churn in the repository.

right, trying to do otherwise gave me an error about wanting exactly 2.64.  so 
i installed that version but still saw the churn.  i'll include the changes in 
the next patch so you can see what i mean.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2] sim: use socklen_t with accept()

[Tom Tromey]

>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

Mike> right, trying to do otherwise gave me an error about wanting
Mike> exactly 2.64.  so i installed that version but still saw the
Mike> churn.  i'll include the changes in the next patch so you can see
Mike> what i mean.

Yeah, I tried it here and see the same diff.
Maybe the previous commit was with the wrong version, it happens
sometimes :)

Tom

Re: [PATCH v3] sim: use socklen_t with accept()

[Tom Tromey]

>>>>> "Mike" == Mike Frysinger <vapier@gentoo.org> writes:

Mike> 2010-03-30  Mike Frysinger  <vapier@gentoo.org>
Mike> 	* configure.ac: Check for socklen_t.
Mike> 	* configure, config.in: Regenerated.
Mike> 	* dv-sockser.c: Typedef socklen_t as int when !HAVE_SOCKLEN_T.
Mike> 	(connected_p): Change addrlen type to socklen_t.

This is ok.  Thanks.

Tom