Rich Megginson wrote:
> The definition of LBER_SOCKET in lber.h:
> #ifndef macintosh
> #if defined( _WINDOWS ) || defined( _WIN32) || defined( _CONSOLE )
> #include <winsock.h> /* for SOCKET */
> typedef SOCKET LBER_SOCKET;
> #else
> typedef long LBER_SOCKET;
> #endif /* _WINDOWS */
> #else /* macintosh */
> typedef void *LBER_SOCKET;
> #endif /* macintosh */
>
> (as an aside, what is a socket/file descriptor on mac? Why a void *?)
A void * is used on Mac OS for historical reasons. In the old days with
Mac OS classic, the TCP stack was not so standardized and most
programmers had their own layer on top of either MacTCP or
OpenTransport... and in many cases a pointer was used (hence the void *
to cover that case).
A long is used on the Unix-y platforms for historical reasons. Probably
something about platforms where integers are 16 bits (I hope none of
those platforms matter much anymore). But the idea of defining a
typedef for LBER_SOCKET in the first place was so most of the code would
not need to know what type it was....
> This should correspond to the native socket/file descriptor type on the
> platform, and is used as such (lber-int.h):
>
> struct sockbuf {
> LBER_SOCKET sb_sd;
> ...
> LBER_SOCKET sb_copyfd; /* for LBER_SOCKBUF_OPT_TO_FILE* opts */
> ...
>
> On every unix/linux platform, the native socket/file descriptor type is
> a 32bit int. So, I propose we change LBER_SOCKET to int.
That sounds OK to me, although I worry the change may cause problems for
some people (probably not major ones though). I assume Mac OS X uses an
int. What about Win32 and Win64? Or are you proposing to change to int
only for the Unix-y platforms?
> In addition, we don't use the type LBER_SOCKET consistently. There are
> a number of places where we use the value of sb_sd such as the following
> (liblber/io.c):
> if ( sb->sb_ext_io_fns.lbextiofn_read != NULL ) {
> rc = sb->sb_ext_io_fns.lbextiofn_read(
> sb->sb_sd, sb->sb_ber.ber_buf,
> ((sb->sb_options & LBER_SOCKBUF_OPT_NO_READ_AHEAD)
> && (len < READBUFSIZ)) ? len : READBUFSIZ,
> sb->sb_ext_io_fns.lbextiofn_socket_arg );
>
> However, the definition of sb->sb_ext_io_fns.lbextiofn_read() is this:
> typedef int (LDAP_C LDAP_CALLBACK LDAP_X_EXTIOF_READ_CALLBACK)( int s,
> void *buf, int bufsize, struct lextiof_socket_private *socketarg );
>
> The first argument should really be LBER_SOCKET s instead of int s.
Yes, although there was some reason we made it an int. I cannot
remember the details right now though.
> Finally, there are a few places where we should use casts to make clear
> what is going on e.g. ldappr-public.c:
> int LDAP_CALL
> prldap_import_connection (LDAP *ld)
> {
> int rc = LDAP_SUCCESS; /* optimistic */
> int shared = 1; /* Assume shared init */
> LBER_SOCKET orig_socket = -1;
> PRLDAPIOSessionArg *prsessp = NULL;
> PRLDAPIOSocketArg *prsockp = NULL;
> PRFileDesc *pr_socket = NULL;
> ...
> /* Retrieve TCP socket's integer file descriptor */
> if ( ldap_get_option( ld, LDAP_OPT_DESC, &orig_socket ) < 0 ) {
> ...
> /* Import file descriptor of connection made via ldap_init() */
> if (NULL == (pr_socket = PR_ImportTCPSocket(orig_socket)) ) {
> ldap_set_lderrno( ld, LDAP_LOCAL_ERROR, NULL, NULL );
>
> The definition of PR_ImportTCPSocket() is this:
> NSPR_API(PRFileDesc*) PR_ImportTCPSocket(PRInt32 osfd);
>
> I suppose we don't need the cast (PRInt32)orig_socket if we change
> LBER_SOCKET to int, but it is of a different type nonetheless.
The only danger in adding casts is if LBER_SOCKET was defined as some
non-integer compatible type the compiler will catch it right now. With
the casts added it may not.
--
Mark Smith
Pearl Crescent
http://pearlcrescent.com/
_______________________________________________
dev-tech-ldap mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-ldap
