i found this comment in my changelog and i should have really got
that documented more verbose and clearly somewhere as i can not
remember now what exactly it was all about. this of course was
done on then private Sun branch so it gets more confusing in
relation to the current merged code.

                 make LBER_SOCKET type long as it used to be.
                this is needed to support *PRFileDesc and some
                trickery done by :
                nslberi_extread_compat(),
                nslberi_extwrite_compat(),
                nslberi_install_compat_io_fns()
                for when prldap/nspr layers are in place and liblber
                users use related liblber interfaces directly as
                opposed to calling into lib[pr]ldap.

since you been looking at all that code recently and have fresh
big picture in your mind about it does above comment ring any
bells in relation to the current code ? if not then its likely
to be irrelevant now. i agree tho that we should make use LBER_
SOCKET consistently everywhere.

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 *?)
> 
> 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.
> 
> 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.
> 
> 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.
> _______________________________________________
> dev-tech-ldap mailing list
> [email protected]
> https://lists.mozilla.org/listinfo/dev-tech-ldap
_______________________________________________
dev-tech-ldap mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-tech-ldap

Reply via email to