[PATCH] samba-tool ntacl do a proper VFS connect
Andrew Bartlett
abartlet at samba.org
Mon Jan 7 14:02:11 MST 2013
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!
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
More information about the samba-technical
mailing list