Re: commit: ldap/libraries/liblber io.c

[Howard Chu <hyc@symas.com>]

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.

--
  -- Howard Chu
  CTO, Symas Corp.           http://www.symas.com
  Director, Highland Sun     http://highlandsun.com/hyc/
  Chief Architect, OpenLDAP  http://www.openldap.org/project/