RE: try_read1msg()/wait4msg() vs. buffered input (ITS#2153)

[hyc@highlandsun.com]

It now occurs to me that just resetting the BerElement using ber_init
introduces a memory leak. The liblber/io.c in OpenLDAP 2.1 has a
ber_free_buf() function that should be used before the call to
ber_init_w_nullc(). Or, since it's only saving one free/malloc invocation,
you may just forget about this whole ber_init idea. Instead of
	ber_init_w_nullc();
	goto retry2;
use
	ber_free( ber, 1 );
	goto retry;
and leave it at that, no need for retry2...

  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support

> -----Original Message-----
> From: Jutta Degener [mailto:jutta@sendmail.com]
> Sent: Thursday, October 24, 2002 2:26 PM
> To: Howard Chu
> Cc: openldap-its@OpenLDAP.org
> Subject: Re: try_read1msg()/wait4msg() vs. buffered input (ITS#2153)
>
>
> Howard,
>
> here's what I translated your patch to, relative to the 2.0.27
> release (I thought we were running 2.0.25, but that was a
> misunderstanding; I was really looking at 2.0.27 all along.)
>
> The only interesting part is that ber_init2 had to be replaced
> with ber_init_w_nullc.  We tested that, and it seems to work
> okay so far; the situation that previously reliably locked
> up now no longer does it, and nothing else has fallen over
> in exchange.
>
> Thank you so much for the quick patch, that was great!
>
> Jutta Degener <jutta@sendmail.com>
> --
> Index: result.c
> ===================================================================
> RCS file: /cvs/pkgs/openldap/src/libraries/libldap/result.c,v
> retrieving revision 1.1.1.3
> diff -c -r1.1.1.3 result.c
> *** result.c	29 Jan 2002 18:45:18 -0000	1.1.1.3
> --- result.c	24 Oct 2002 18:32:17 -0000
> ***************
> *** 359,364 ****
> --- 359,365 ----
>
>   	Debug( LDAP_DEBUG_TRACE, "read1msg: msgid %d, all
> %d\n", msgid, all, 0 );
>
> + retry:
>       if ( lc->lconn_ber == NULL ) {
>   		lc->lconn_ber = ldap_alloc_ber_with_options(ld);
>
> ***************
> *** 370,375 ****
> --- 371,377 ----
>   	ber = lc->lconn_ber;
>   	assert( BER_VALID (ber) );
>
> + retry2:
>   	/* get the next message */
>   	errno = 0;
>   	if ( (tag = ber_get_next( sb, &len, ber ))
> ***************
> *** 407,414 ****
>
>   	/* if it's been abandoned, toss it */
>   	if ( ldap_abandoned( ld, id ) ) {
> - 		ber_free( ber, 1 );
>   		Debug( LDAP_DEBUG_ANY, "abandoned\n", 0, 0, 0);
>   		return( -2 );	/* continue looking */
>   	}
>
> --- 409,422 ----
>
>   	/* if it's been abandoned, toss it */
>   	if ( ldap_abandoned( ld, id ) ) {
>   		Debug( LDAP_DEBUG_ANY, "abandoned\n", 0, 0, 0);
> + retry_ber:
> + 		if ( ber_sockbuf_ctrl( sb,
> LBER_SB_OPT_DATA_READY, NULL ))
> + 		{
> + 			ber_init_w_nullc( ber, ld->ld_lberoptions );
> + 			goto retry2;
> + 		}
> + 		ber_free( ber, 1 );
>   		return( -2 );	/* continue looking */
>   	}
>
> ***************
> *** 416,423 ****
>   		Debug( LDAP_DEBUG_ANY,
>   		    "no request for response with msgid %ld
> (tossing)\n",
>   		    (long) id, 0, 0 );
> ! 		ber_free( ber, 1 );
> ! 		return( -2 );	/* continue looking */
>   	}
>
>   	/* the message type */
> --- 424,430 ----
>   		Debug( LDAP_DEBUG_ANY,
>   		    "no request for response with msgid %ld
> (tossing)\n",
>   		    (long) id, 0, 0 );
> ! 		goto retry_ber;
>   	}
>
>   	/* the message type */
> ***************
> *** 692,698 ****
>
>   		new->lm_next = ld->ld_responses;
>   		ld->ld_responses = new;
> ! 		return( -2 );	/* continue looking */
>   	}
>
>   	Debug( LDAP_DEBUG_TRACE, "adding response id %ld type %ld:\n",
> --- 699,705 ----
>
>   		new->lm_next = ld->ld_responses;
>   		ld->ld_responses = new;
> ! 		goto leave;
>   	}
>
>   	Debug( LDAP_DEBUG_TRACE, "adding response id %ld type %ld:\n",
> ***************
> *** 729,734 ****
> --- 736,745 ----
>   #endif	/* !LDAP_WORLD_P16 */
>   	}
>
> + leave:
> + 	if ( ber_sockbuf_ctrl( sb, LBER_SB_OPT_DATA_READY, NULL ) ) {
> + 		goto retry;
> + 	}
>   	return( -2 );	/* continue looking */
>   }
>