svn commit: samba r17372 - in branches/SOC/mkhl/ldb-map/modules: .

[Andrew Bartlett]

On Thu, 2006-08-03 at 18:10 +0200, Martin Kühl wrote:
> On 8/2/06, abartlet at samba.org <abartlet at samba.org> wrote:
> 
> I am fine with most of this changeset, however...
> 
> > --- branches/SOC/mkhl/ldb-map/modules/ldb_map.c 2006-08-02 01:25:05 UTC (rev 17371)
> > +++ branches/SOC/mkhl/ldb-map/modules/ldb_map.c 2006-08-02 02:24:00 UTC (rev 17372)
> [...]
> > @@ -2046,6 +2072,7 @@
> >                                 ldb_msg_add(msg, el, 0);
> >                 }
> >
> > +               msg->dn = talloc_steal(msg, ares->message->dn);
> >                 talloc_free(ares->message);
> >                 ares->message = msg;
> >         }
> 
> This change caused a strange regression in samba3sam: Add requests
> started to fail because the second search in `samldb_get_new_sid'
> started yielding bogus results.  It turned out that the name of the
> "objectSid" element of the result got corrupted (and read "objectSi@"
> instead).  This happened regardless of whether the former message was
> freed or not (`ares->message' from the above snippet), but stopped
> when I stopped stealing the DN...

Until I did this, it caused the DN in the result to be NULL in the
ldbsearch, because nothing filled it in.

> I remembered the `ldb_msg_remove_attr' function though and am about to
> replace this block with it.

OK.

> > @@ -2242,9 +2269,8 @@
> >
> >         /* XXX: ugly kludge
> >         ac->local_attrs = local_attrs;
> > +       */
> >         ac->remote_req->op.search.attrs = remote_attrs;
> > -       */
> > -       ac->remote_req->op.search.attrs = NULL;
> >
> >         /* split local from remote tree */
> >         ret = partition_tree(module, ac->remote_req, ac,
> 
> When checking a search result against the original parse-tree, I
> wanted to have all attributes of the record available, which is why I
> set the searched attributes to NULL here.
> Can you tell me the reason you wanted this change?

I need to get at additional attributes, which are operational, and
therefore don't show up until you ask for them.

> > @@ -2672,6 +2698,13 @@
> >         ac->remote_req->op.del.dn = map_local_dn(module, ac->remote_req,
> >                                                  req->op.del.dn);
> >
> > +       /* The DN didn't change, so just pretend we were never here */
> > +       if (ldb_dn_compare(module->ldb, ac->remote_req->op.del.dn,
> > +                          req->op.del.dn) == 0) {
> > +               talloc_free(h);
> > +               return ldb_next_request(module, req);
> > +       }
> > +
> >         ac->remote_req->context = NULL;
> >         ac->remote_req->callback = NULL;
> >
> 
> This change causes another regression samba3sam, where it is common
> that the local and remote DN are equal until the remote one is
> rebased.  I think we should test equality of the base DNs instead to
> determine whether to run the local requests at all.

Yeah, I suspect I was just looking for a quick hack to avoid touching
the local database, but didn't consider this possibility.

> > @@ -2774,10 +2807,11 @@
> >         if (ldb_dn_is_special(req->op.rename.olddn))
> >                 return ldb_next_request(module, req);
> >
> > -       /* no mapping requested, skip to next module */
> > -       if (!check_dn_local(module, req->op.rename.olddn) &&
> > -           !check_dn_local(module, req->op.rename.newdn))
> > +       /* no mapping requested, (perhaps no DN mapping specified), skip to next module */
> > +       if ((!check_dn_local(module, req->op.rename.olddn) &&
> > +            !check_dn_local(module, req->op.rename.newdn))) {
> >                 return ldb_next_request(module, req);
> > +       }
> >
> >         /* rename into/out of the mapped partition requested, bail out */
> >         if (!check_dn_local(module, req->op.rename.olddn) ||
> 
> Um, you just wrapped the whole expression the `if' tests in another
> pair of parens.  Did you perhaps mean to wrap the subexpressions
> instead? :-)

Yeah, sorry about that.  I added another expression, then removed it.

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Samba Developer, Red Hat Inc.                  http://redhat.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20060804/8d47bea7/attachment.bin