ldap_parse_result assert() (ITS#1859)

[argggh@linpro.no]

Full_Name: Arne Georg Gleditsch
Version: 2.0.23
OS: GNU/Linux
URL: 
Submission from: (NULL) (213.203.57.142)


ldap_parse_result in error.c contains the code

        assert( ld != NULL );
        assert( LDAP_VALID( ld ) );
        assert( r != NULL );

        if ( ld == NULL || r == NULL ) {
                return LDAP_PARAM_ERROR;
        }

What is the point of this, really?  (The same kind of belt-and-suspenders
redundant code can be found elsewhere in (at least) errors.c as well.)

I've just had mail bounce because my ldap server was hit too hard and returned
bogus data (in some form) to the ldap client, in this case exim.  (This is 
our reconstrution of the chain of events, at least.  Sorry, I don't have any
real bug reports for slapd on this one.)

What I _do_ have is a bounce saying:

  This is a permanent error; I've given up. Sorry it didn't work out.

  <xx@xx.xx>:
  <my exim ip> does not like recipient.
  Remote host said: exim: error.c:221: ldap_parse_result: Assertion `r != ((void
  *)0)' failed.
  Giving up on <my exim ip>.

And I'm not happy about it.  (Let us ignore the fact that "does not like
recipient" seems to be an unwarranted conclusion in this case and stay with the
issues in our
end.)  Firstly, I think letting Exim handle the LDAP_PARAM_ERROR would have
been
clearly preferable, but secondly calling assert at this point has the side
effect of
causing noise to be printed on fd 1, which in this case is Exim's end of the
current
SMTP dialog.  Bad.

Spewing noise on file descriptors we know nothing about and rolling over to die
in a library should, IMHO, be reservered for extreme cases where the internal
state of the library is no longer reliable, if at all.  Is this the case here,
or
could ldap_parse_result be modified to just return LDAP_PARAM_ERROR?


Arne.