ACL module patches

[Nadezhda Ivanova]

About the contexts I would prefer too to have tmp. Matthias is right however
that some of them are quite scrambled.
I will go over the entire module again to make sure all of them have the
right parents and are freed when necessary.

On Wed, Jul 7, 2010 at 1:23 AM, Andrew Bartlett <abartlet at samba.org> wrote:

> On Tue, 2010-07-06 at 20:50 +0300, Nadezhda Ivanova wrote:
> > Hi Matthias,
> > I basically made sure to do what you did in password_hash, and I did not
> see
> > this control there, but it looks all right. As for the memory context
> > patches - I am not so sure. They will not have any effect but as far as I
> > know contexts should be as granular as possible, otherwise you risk a
> method
> > freeing a context that was passed as parameter instead of the local one.
> And
> > the habit of moving line endings around even if they do not exceed the
> > character limit... Well, in short, I am not convinced that the second
> patch
> > is necessary, but I am not against applying it. However, since they are
> your
> > patches, I think you should push them :). Just make sure to run both
> acl.py
> > and password.py tests before that.
>
> I've made some comments below:
>
> > Regards,
> > Nadya
> >
> > On Tue, Jul 6, 2010 at 7:55 PM, Matthias Dieter Wallnöfer <mdw at samba.org
> >wrote:
> >
> > > Hi Nadya,
> > >
> > > the ACL password work does work well beside one exception of which you
> > > probably wasn't aware. I put here the link to the patch from which the
> > > commit message should explain the reason:
> > >
> > >
> http://repo.or.cz/w/Samba/mdw.git/commitdiff/265a2ea034ceb2c29d075933516bbdf6f042c9c9
>
> When samdb_check_password is called in the cases indicated (user
> password changes over samr or kpasswd) isn't the system_session() used
> anyway?
>
> This looks like a reasonable change, but I don't think it's a sufficient
> change to actually test this.
>
> > > The other patch should fix the outstanding memory contexts:
> > >
> http://repo.or.cz/w/Samba/mdw.git/commitdiff/b3880b5f25b88b524cbb2b5768e37b83109976a1
> .
> > > Would be nice if you could integrates this one too.
> > >
>
> I really don't like this patch.  We should use tmp_ctx more, not less,
> in a complex module such as this.


> Andrew Bartlett
>
> --
> Andrew Bartlett                                http://samba.org/~abartlet/<http://samba.org/%7Eabartlet/>
> Authentication Developer, Samba Team           http://samba.org
> Samba Developer, Cisco Inc.
>