ACL module patches

Matthias Dieter Wallnöfer mdw at samba.org
Wed Jul 7 09:56:40 MDT 2010


Hi Nadya,

Nadezhda Ivanova wrote:
> 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.
please do that - then I'm fine.

Greets,
Matthias

>
> On Wed, Jul 7, 2010 at 1:23 AM, Andrew Bartlett <abartlet at samba.org 
> <mailto: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 <mailto: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.
>
>



More information about the samba-technical mailing list