Bugs item #1733119, was opened at 2007-06-08 00:43
Message generated for change (Comment added) made by bagder
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1733119&group_id=976

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: libcurl
Group: wrong behaviour
Status: Open
Resolution: None
Priority: 6
Private: No
Submitted By: rrauenza (rrauenza)
Assigned to: Daniel Stenberg (bagder)
Summary: socket.c assumes xopen getsockopt/getsockname

Initial Comment:

HPUX doesn't use the XOPEN getsockopt/getsockname interfaces by default (for backwards compatibility).

     int setsockopt(
          int s,
          int level,
          int optname,
          const void *optval,
          int optlen
      );

    _XOPEN_SOURCE_EXTENDED Only (UNIX 98)
      int getsockopt(
          int s,
          int level,
          int optname,
          void *optval,
          socklen_t *optlen
      );

When compiling 64bit, socklen_t is 64bit, but int is 32bit, so optlen ends up being 0 due the socklen being treated as a 32bit value.

Here's the required changes -- simply changing the parms to be ints instead of socklens when not using _XOPEN_SOURCE_EXTENDED and HPUX

$ rcsdiff connect.c
RCS file: connect.c,v
retrieving revision 1.1
rdiff -r1.1 connect.c
367a368,370
> #if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
> int size;
> #else
368a372
> #endif
407a412,414
> #if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
> int errSize = sizeof(err);
> #else
408a416
> #endif

----------------------------------------------------------------------

>Comment By: Daniel Stenberg (bagder)
Date: 2007-06-11 15:51

Message:
Logged In: YES
user_id=1110
Originator: NO

The configure script already checks for an existing socklen_t type and if
it doesn't exist, it provides an alternative. What does your lib/config.h
say about socklen_t when configure has run?

If you make sure it does a '#define socklen_t int' in there, does that
make the build work?

----------------------------------------------------------------------

Comment By: rrauenza (rrauenza)
Date: 2007-06-09 00:41

Message:
Logged In: YES
user_id=1309815
Originator: YES

No, it can't be fixed by just defining _XOPEN_SOURCE_EXTENDED in curl's
configure because some applications linking with curl are compiled without
it. (The one I found this bug with is has this exact case).

If my application uses the BSD interfaces, it would not work with curl if
curl was compiled with _XOPEN_SOURCE_EXTENDED. It gets sticky because some
OTHER 3rd party dependencies may or may not have XOPEN_SOURCE defined and
you have to find the common denominator.. or where it breaks the least.
Best compatibility is to allow both modes of compilations by end user
(later versions of HPUX has a fix for this to allow mixed linkages.. but it
won't be on the earlier OS's you may wish to support.)

This doesn't break on other platforms because socklen is always a 32bit
value. For some reason HPUX typedefs socklen to be a size_t, which is
64bit in 64bit ABI's.

If there is a common header you might do something like..

#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
#define socklen_t int
#endif

but I don't know offhand if that could break anything else... outside of
system include files, of course.

Here's more context of my changes in socket.c -- this is really only a
problem for the socket calls that take a pointer to a socklen_t. The
others are passed by value and the compiler just casts the 64bit value down
to an int.

Interestingly, krb4.c works ok because it uses an int:

  int l = sizeof(conn->local_addr);
  struct SessionHandle *data = conn->data;
  CURLcode result;

  if(getsockname(conn->sock[FIRSTSOCKET],
                 (struct sockaddr *)LOCAL_ADDR, &l) < 0)
    perror("getsockname()");

Diff's:

$ /usr/local/bin/diff -u ftp.c ftp.c.orig
--- ftp.c 2007-06-08 15:34:22 -0700
+++ ftp.c.orig 2007-06-08 15:28:09 -0700
@@ -222,12 +222,7 @@
 #else
       struct sockaddr_in add;
 #endif
-
-#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
- int size = sizeof(add);
-#else
       socklen_t size = (socklen_t) sizeof(add);
-#endif
 
       if(0 == getsockname(sock, (struct sockaddr *) &add, &size)) {
         size = sizeof(add);
@@ -819,11 +814,7 @@
    */
   struct Curl_sockaddr_storage ss;
   struct addrinfo *res, *ai;
-#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
- int sslen;
-#else
   socklen_t sslen;
-#endif
   char hbuf[NI_MAXHOST];
   struct sockaddr *sa=(struct sockaddr *)&ss;
   char tmp[1024];
@@ -1122,11 +1113,7 @@
       if(bind(portsock, (struct sockaddr *)&sa, sslen) == 0) {
         /* we succeeded to bind */
         struct sockaddr_in add;
-#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
- int socksize = sizeof(add);
-#else
         socklen_t socksize = sizeof(add);
-#endif
 
         if(getsockname(portsock, (struct sockaddr *) &add,
                        &socksize)) {
$ /usr/local/bin/diff -u connect.c connect.c.orig
--- connect.c 2007-06-08 15:25:36 -0700
+++ connect.c.orig 2007-06-08 15:25:51 -0700
@@ -365,11 +365,7 @@
     if( bind(sockfd, sock, socksize) >= 0) {
       /* we succeeded to bind */
       struct Curl_sockaddr_storage add;
-#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
- int size;
-#else
       socklen_t size;
-#endif
 
       size = sizeof(add);
       if(getsockname(sockfd, (struct sockaddr *) &add, &size) < 0) {
@@ -409,11 +405,7 @@
   bool rc = TRUE;
 #ifdef SO_ERROR
   int err = 0;
-#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
- int errSize = sizeof(err);
-#else
   socklen_t errSize = sizeof(err);
-#endif
 
 #ifdef WIN32
   /*

Another option could be something like this where hp_getsockname does the
cast/copy from socklen_t to temp int before passing the ptr..

#if defined(__hpux) && ! defined(_XOPEN_SOURCE_EXTENDED)
#define getsockname(a,b,c) hp_getsockname(a,b,c)
#define getsockopt(a,b,c) hp_getopt(a,b,c)
#endif

----------------------------------------------------------------------

Comment By: Daniel Stenberg (bagder)
Date: 2007-06-08 10:10

Message:
Logged In: YES
user_id=1110
Originator: NO

Can you please submit the patch done with diff -u ? I'm having trouble
reading it unless there's some context in it as well.

Is there any particular reason why we shouldn't just try to define
_XOPEN_SOURCE_EXTENDED properly and make your system act/work like other
systems instead? I'd really hate to add such a kludgey #ifdef mess in at
least three different source files. It'll also just beg for this to happen
the next time we add such a function call or modify one of the existing.

I would really prefer a way that cures the problem without relying on
#ifdefs where getsockopt() is used. You think that is possible?

----------------------------------------------------------------------

Comment By: rrauenza (rrauenza)
Date: 2007-06-08 01:58

Message:
Logged In: YES
user_id=1309815
Originator: YES

Looks like ftp.c and tftp.c also need to be fixed. Anywhere those calls
are made.

----------------------------------------------------------------------

You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=100976&aid=1733119&group_id=976
Received on 2007-06-11