Re: OpenLDAP/NT error handling

["Kurt D. Zeilenga" <Kurt@OpenLDAP.org>]

Paul wrote:
>I've been trying to resolve some issues around the abnormal disconnection of
>clients and have really come to a temporary dead end. The problem lies
>primarily in error handling from the socket functions and the use of errno
>all through the source code.

Library use:
Per my reading of the API draft spec, ldap routines should return
non-protocol errors through the operating system error reporting
mechanism.

Under UNIX, we can just set errno.   Under NT, there are multiple
error reporting mechanisms (errno and WSA error routines).  This
is a problem as users need to know which to check.  A call like
ldap_open() could have error needing to be reported via errno
(such as ENOMEM) or a socket error (such as WSAEMFILE).  When
should the application check errno and when should it check
WSALastError()?

>In order to make a really good "single source" distribution of OpenLDAP that
>works on with Unix sockets and NT sockets, I believe that we need to
>abstract the socket error handling functions in some way.

I concur.

One approach would be set errno=EIO when a socket error occured.
Applications could then check errno first and if EIO check
the WSA error.  A helper routines could be provided,
	int lber_get_sys_error(void);
	int lber_get_net_error(void);
	int lber_get_error(void);
	void lber_clr_sys_error(void);
	void lber_clr_net_error(void);
	void lber_clr_error(void);

Internally, we probably should use a couple of private routines
to localize error setting.
	lber_int_set_sys_error();
	lber_int_set_net_error();

Another approach would be to change the spec such that all
routines directly returned the error codes.  In the long run,
I think this actually the best approach.  (Maybe we can draw
from the Pthread specification process on this... they changed
error handling mid-stream through standardization process).

>If ALL the code base used these macros then evaluating the results of error
>functions would be much easier. To add another layer, we could abstract the
>error codes into LDAP_ERR_xxxxx values.

The problem here is that every platform has different error code sets.
Hence, I think should only abstract those to which the code actually
depends upon (like EWOULDBLOCK vs WSAEWOULDBLOCK).


A few comments on your suggestions:

>//---- inclusion - portable_err.h -------
>// includes

This should be handled in either <ac/errno.h>
or <ac/socket.h>.  General error codes should
probably go in <ac/errno.h> and socket errors
in <ac/socket.h>.  (Actually, I'd just put everything
in <ac/errno.h>)

Use Standard C comments, ie: /* comment */

>#ifdef WIN32
Should use HAVE_WINSOCK2_H instead.

>#include <winsock2.h>
We may want to avoid including winsock2.h in <ac/errno.h>.

>#else
>#include <errno.h>
Errno.h should be included under NT to get common errors.
>#endif

>// some common abstract error codes
>#define LDAP_PVT_ERR_AGAIN					EAGAIN
>#define LDAP_PVT_ERR_RANGE					ERANGE
I think we should use common error codes directly.

>// abstract error codes 
>#ifdef WIN32
>#define LDAP_PVT_ERR_ACCES					 WSAEACCES 
>#define LDAP_PVT_ERR_ADDRINUSE				 WSAEADDRINUSE 
>...
Too many to maintain.  Why not just add

#ifdef HAVE_WINSOCK
#define	EWOULDBLOCK	WSAEWOULDBLOCK
#endif

(and the few others we need) to either ac/errno.h or ac/socket.h.

>
>#ifdef WIN32
>#define ldap_pvt_setsockerr( err ) \
>	WSASetLastError(err)
>#else
>#define ldap_pvt_setsockerr( err ) \
	do { errno = err } while(0)
>#endif
Something like this could be added to <ac/socket.h>

>#ifdef WIN32
>#define ldap_pvt_getlasterr() \
>	GetLastError()
>#else
>#define ldap_pvt_getlasterr() \
>	errno
>#endif

When should [GS]etLastError(err) be used in favor of errno?


>#define __RETSTR2( x, y ) case x: return #x"("y")";
>char *ldap_pvt_getsockerrstr( int err )
>{
>#ifdef WIN32
>	switch( err )
>	{
>		__RETSTR2( LDAP_PVT_ERR_INTR, "WSAEINTR" )

If the vendor doesn't provide error strings, I think we should
just return the value of "WSA ERROR %d" or something.  I rather
not have to maintain a error code to string mapping.

Kurt