Re: commit: ldap/servers/slapd/back-ldbm dn2id.c

[Pierangelo Masarati <masarati@aero.polimi.it>]

> > -----Original Message-----
> > From: Pierangelo Masarati [mailto:masarati@aero.polimi.it]
> 
> > > I believe your comment is unnecessary/misleading. The call to dn_parent
> > > occurs after the check for be_issuffix. The be_issuffix check 
> > is needed to
> > > avoid creating a DN_SUBTREE_PREFIX index for the backend suffix. For
> > > example,
> > > 	be_suffix: o=foo
> > > 	new dn: ou=zip,o=foo
> > > The code will create a DN_SUBTREE_PREFIX index for ou=zip,o=foo
> > > It will then call dn_parent(ou=zip,o=foo) and get o=foo.
> > > It will then create a DN_ONE_PREFIX index for o=foo.
> > > It then must not create a DN_SUBTREE_PREFIX for o=foo. The 
> > be_issuffix check
> > > at that point is required, cannot be optimized out.
> > 
> > I mean: if pdn is one level below the suffix, be_issuffix(be, 
> > pdn) will fail;
> > the next call to dn_parent will return NULL, so there won't ever 
> > be a successful 
> > call to be_issuffix(be, pdn).
> 
> If that is true, then there is now a bug. Again with the example above,
>   dn_parent(ou=zip,o=foo) should return o=foo, not NULL. Otherwise the
> DN_ONE_PREFIX will not be created correctly for the backend suffix.

Actually I was going to ask if that check in dn_parent was neded at all.
This is the code before my reworking:

/*
 * dn_parent - return the dn's parent, in-place
 */
char *
dn_parent(
	Backend *be,
	const char      *dn )
{
	const char      *s;
	int     inquote;

	if( dn == NULL ) {
		return NULL;
	}

	while(*dn != '\0' && ASCII_SPACE(*dn)) {
		dn++;
	}

	if( *dn == '\0' ) {
		return NULL;
	}

	if ( be != NULL && be_issuffix( be, dn ) ) {
		return NULL;
	}

	// ...

In this case, we'd better eliminate all of this.  Note that this implies
we can eliminate also all the cleanup before calling ldap_str2rdn, because
it is already performed by the parsing stuff.  I left it in place only
to allow the be_issuffix() check :)

Ando.