(ITS#5304) bug in source code: libldap/extended.c ?

[mark.funkenhauser@bluecoat.com]

Full_Name: Mark Funkenhauser
Version: 
OS: 
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (216.16.247.2)


Looking at libldap/extended.c, rev 1.43, function ldap_parse_intermediate(),
at line 374, the code is written as:
	if ( ber_scanf( ber, /*{*/ "}" ) == LBER_ERROR ) {
		rc = LDAP_DECODING_ERROR;
		goto free_and_return;
	}
I think there's a problem here.

1) rc is defined to be a ber_tag_t type, which is not suitable for
   assignment to an LDAP_*_ERROR error code.
   ber_tag_t is an unsigned type, and LDAP_DECODING_ERROR is a negative number.

2) The use of LDAP_DECODING_ERROR here implies there is an error.
   I would have thought this function should be returning this error code
   using "return ld->ld_errno".
   Jumping to "free_and_return" is not the way to return an error.

3) the "goto free_and_return" is a jump to code to handle a successful
   return from this function.
   Assigning any value to rc before this "goto" becomes a no-op because
   the code after "free_and_return" does not use rc.

I think the code at line 374 should look like:
	if ( ber_scanf( ber, /*{*/ "}" ) == LBER_ERROR ) {
		ld->ld_errno = LDAP_DECODING_ERROR;
		ber_free( ber, 0 );
		if( resoid != NULL ) LDAP_FREE( resoid );
		return ld->ld_errno;
	}