[PATCH] s4-drs: Synchronous Implementation of generated ParentGUID - ToDo List ParentGUID fix

[Fernando J V da Silva]

Hi Tridge!


Thanks for reply!

It looks like the new patch you sent is a delta against your earlier
> patch. The usual approach is to only send a delta patch if your
> earlier patch had already been accepted into the tree. If none of your
> patch has been accepted yet, then send the full patch.
>

Sorry for that! I thought it should be incremental. I just sent the 3
patches the way you asked for!


>  > +/*
>  > +  use a DN to find it's parentGUID
>  > + */
>  > +int dsdb_find_parentguid_by_dn(struct ldb_context *ldb,
>  > +                    struct ldb_dn *dn, struct GUID **parent_guid)
>  > +{
>
> why does this take a struct GUID ** ? That would seem to imply that it
> returns allocated memory, but if you want to return allocated memory
> then you should pass in a memory context to put it on. Currently you
> allocate it as a child of the ldb, which could lead to memory leaks.
>
> I think it would be better if this function took a "struct GUID *" and
> left it up to the caller to allocate the GUID (perhaps just as a stack
> variable).
>
>
I was trying this way because I was returning a NULL GUID* when the object
doesn't have a parent, so the caller could know when there is just no
parentGUID and when an error happened.
Now I changed the approach: If the object doesn't have a parent, then it
returns LDB_ERR_NO_SUCH_OBJECT and when it isn't possible to get the
objectGUID of the parent, it returns LDB_ERR_NO_SUCH_ATTRIBUTE. Do you think
this approach is a suitable solution?


>  > +/*
>  > +  Generate the parentGUID and adds it to the message
>  > + */
>  > +int dsdb_msg_add_parentguid(struct ldb_context *ldb, struct ldb_message
> *msg)
>
> I don't think this is a good function for util.c. Things that go into
> util.c should be more generic.
>
> Instead I'd suggest you have this function:
>
>  int dsdb_msg_add_guid(struct ldb_context *ldb, struct ldb_message *msg,
>                        struct GUID *guid);
>
> and then call both dsdb_find_parentguid_by_dn() and
> dsdb_msg_add_guid() from operational.c.
>
>
Sorry (2), I thought that function would help... Now I changed it to
dsdb_msg_add_guid(), but I put the attr_name parameter, which tells the name
of the attribute to be added. That allows to add either objectGUID or
parentGUID to a message. Does it fit to what you had in mind?



> In the above you changed the code from looking for just the
> "objectGUID" attribute to looking for the NULL attribute list. When
> you search for the NULL attribute list in ldb it means that you want
> all attributes, not no attributes. So instead you should pass the
> empty list (which means no attributes).
>

Ok! I changed it!


Best regards,

-- 
Fernando J V da Silva
M Sc Computer Science Student
Institute of Computing, State University of Campinas
+55 15 8801-2165