Hallvard B Furuseth wrote:
Looking closer now that I'm awake...
Howard Chu writes:
Hallvard B Furuseth wrote:
Howard Chu writes:
Keep the sock_errset() if returning LBER_DEFAULT and sblen< 0?
No. ber_int_sb_read() will cause errno to already be set if it cannot fulfill the request.
Sorry, I meant sblen>= 0 of course. That can be a read() result which does not set errno.
No, doesn't matter. A blocking read that returns 0 means of course that the connection was closed.
But the caller doesn't know that. What it knows is that ber_get_next() returned LBER_DEFAULT.
And that's all that matters. If the caller doesn't see one of the try-again error codes, then the connection is dropped. In the case of read returning 0, that means the connection was closed by the remote side anyway.
Anyway, that part of the code seems to be gone now.
A non-blocking read that returns 0 will set errno.
Not on my host (Linux). And not in the Posix spec that I can see.
My mistake; actually a non-blocking read will return -1 (with errno set) when there's no data available. A return of 0 always means the connection was closed.
In any case, it looks like the error number handling is incomplete:
ber_get_next() can do sock_errset(0) before calling ber_int_sb_read() and returning LBER_DEFAULT if zero result, however that function can set errno spuriously and then return zero. At least in an EINTR loop there or in sb_rdahead_read(). I don't know if the read OS call itself is one of the functions which can set errno (or WSASetLastError) spuriously on some OSes.
Irrelevant.
ber_get_next() can return LBER_DEFAULT due to memory allocation error, I don't suppose that does sock_errset(). Haven't checked if all variants set errno either before failing.
Irrelevant. Anything besides EAGAIN/EWOULDBLOCK (including 0) is a fatal error and the connection is dropped.
On the caller side, slapd/connection.c treats all other return values than LDAP_TAG_MESSAGE as LBER_DEFAULT and examines sock_errno().
Yes.
libldap/result.c does Debug() before reading sock_errno(). Debug() can change errno.
That should probably be changed.