[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: OpenLDAP/NT error handling

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

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

>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.

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>.

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

>// some common abstract error codes
I think we should use common error codes directly.

>// abstract error codes 
>#ifdef WIN32
Too many to maintain.  Why not just add


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

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

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

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 )
>	{

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.