[PATCH] samba-tool ntacl do a proper VFS connect

[Jeremy Allison]

On Tue, Jan 08, 2013 at 09:53:00AM +1100, Andrew Bartlett wrote:
> On Tue, 2013-01-08 at 08:02 +1100, Andrew Bartlett wrote:
> > On Mon, 2013-01-07 at 11:25 -0800, Jeremy Allison wrote:
> > > On Fri, Dec 28, 2012 at 04:35:00PM +1100, Andrew Bartlett wrote:
> > > > >From bb140b818d4f76fd51d4a72ce67869b4032fcf83 Mon Sep 17 00:00:00 2001
> > > > From: Andrew Bartlett <abartlet at samba.org>
> > > > Date: Wed, 10 Oct 2012 13:47:49 +1100
> > > > Subject: [PATCH 1/6] smbd: Split create_conn_struct into a fn that does not
> > > >  change the working dir
> > > > 
> > > > The python bindings do not want the current working directory changed
> > > > during operations, so we provide two functions, one providing the
> > > > original behaviour, and other providing the python bindings with just
> > > > the memory allocation and initilisation stuff.
> > > 
> > > I'm perfectly fine with the function name changes, just FYI.
> > > 
> > 
> > > (i.e. you added the snum != -1 check).
> > > 
> > > I can see the point of it, but when splitting a function
> > > in two the cleaner way of doing it is to simply split it
> > > out in one patch, then add the aditional change as a subsequent
> > > patch.
> > > 
> > > That way it's much clearer to the reviewer what is going on
> > > as you're not mixing logic changes with just moving code.
> > > Small changes like this make it *much* easier to review
> > > changes without missing anything.
> > > 
> > > The rest of the patchset looks fine to me, if you
> > > can separate out this logic change and resubmit I'll
> > > push to master.
> > 
> > Thanks, and sorry for including the -1 change.  It shows the importance
> > of review, because I hadn't intended to leave it in there, it isn't
> > strictly required for the rest of the patch set (but needed for a future
> > round that enforces the use of VFS connect).
> > 
> > I'll address the whitespace and resubmit today.  Thanks for your careful
> > review!
> 
> Attached are the corrected patches.

Looks good. Just running through make test then I'll push.

Jeremy.