[Fwd: Re: [PATCH] s3: Fix max indentation and max column]
Swen Schillig
swen at vnet.ibm.com
Wed Mar 7 07:53:39 UTC 2018
On Tue, 2018-03-06 at 15:22 -0800, Jeremy Allison wrote:
> On Tue, Mar 06, 2018 at 01:15:57PM +0100, Swen Schillig via samba-technical wrote:
> > From 3d418cd2213a3721bd4cc6185ad5d515fcbd724b Mon Sep 17 00:00:00
> > 2001
> > From: Swen Schillig <swen at vnet.ibm.com>
> > Date: Mon, 5 Mar 2018 12:55:23 +0100
> > Subject: [PATCH] s3: Fix max indentation and max column
> >
> > Minor cleanup reducing the max indentation level and max column length.
> >
> > Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
> > ---
> > source3/smbd/dir.c | 43 +++++++++++++++++++++++--------------------
> > 1 file changed, 23 insertions(+), 20 deletions(-)
> >
> > diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c
> > index 6621b4ee387..4fad65c90d7 100644
> > --- a/source3/smbd/dir.c
> > +++ b/source3/smbd/dir.c
> > @@ -159,29 +159,32 @@ static struct dptr_struct *dptr_get(struct smbd_server_connection *sconn,
> > int key, bool forclose)
> > {
> > struct dptr_struct *dptr;
> > + const int dirhandles_open = sconn->searches.dirhandles_open;
> >
> > - for(dptr = sconn->searches.dirptrs; dptr; dptr = dptr->next) {
> > - if(dptr->dnum == key) {
> > - if (!forclose && !dptr->dir_hnd) {
> > - if (sconn->searches.dirhandles_open >= MAX_OPEN_DIRECTORIES)
> > - dptr_idleoldest(sconn);
> > - DEBUG(4,("dptr_get: Reopening dptr key %d\n",key));
> > -
> > - if (!(dptr->dir_hnd = OpenDir(NULL,
> > - dptr->conn,
> > - dptr->smb_dname,
> > - dptr->wcard,
> > - dptr->attr))) {
> > - DEBUG(4,("dptr_get: Failed to "
> > - "open %s (%s)\n",
> > - dptr->smb_dname->base_name,
> > - strerror(errno)));
> > - return NULL;
> > - }
> > + for (dptr = sconn->searches.dirptrs; dptr; dptr = dptr->next) {
> > + if(dptr->dnum != key) {
> > + continue;
> > + }
> > +
> > + if (!forclose && !dptr->dir_hnd) {
> > + if (dirhandles_open >= MAX_OPEN_DIRECTORIES) {
> > + dptr_idleoldest(sconn);
> > + }
> > + DEBUG(4,("dptr_get: Reopening dptr key %d\n",key));
> > +
> > + dptr->dir_hnd = OpenDir(NULL, dptr->conn,
> > + dptr->smb_dname,
> > + dptr->wcard, dptr->attr);
>
> As a formatting comment. Please don't change:
>
> > - if (!(dptr->dir_hnd = OpenDir(NULL,
> > - dptr->conn,
> > - dptr->smb_dname,
> > - dptr->wcard,
> > - dptr->attr))) {
>
> to
>
> > + dptr->dir_hnd = OpenDir(NULL, dptr->conn,
> > + dptr->smb_dname,
> > + dptr->wcard, dptr->attr);
>
> when reformatting. When calling a function one paramter per line is *much* easier to
> read and modify than multiple paramerers per-line.
>
> This is because if you need to change one parameter later, it makes it extremely
> clear what it is you are changing in a future patchset.
>
> I know it seems overly picky, but I'd like to NAK until I see a version
> that looks like:
>
> > + dptr->dir_hnd = OpenDir(NULL,
> > + dptr->conn,
> > + dptr->smb_dname,
> > + dptr->wcard,
> > + dptr->attr);
>
> instead. It's very close to being right, I want to make the
> patch be beautiful as well :-) :-).
Well, you know how it is, beauty depends on the viewer :-)
but here I'm afraid I would code against the SAMBA Coding rules if I'd
do it the way you want.
Since this is an invocation and no declaration or definition of a
function, I have to use the full length.
So, I guess I'm stuck, I won't get a RB+ from you without the re-do but
I might get a NACK from another team-member if I do.
Any suggestions ?
Cheers Swen
More information about the samba-technical
mailing list