[PATCH] Add utilities to check style

Martin Schwenke martin at meltin.net
Mon Apr 23 06:13:18 UTC 2018


On Sat, 21 Apr 2018 17:34:12 -0400, Simo via samba-technical
<samba-technical at lists.samba.org> wrote:

> > On 22/04/18 09:08, Simo via samba-technical wrote:  
> > > New revision of the patch to check style.
> > > 
> > > Martin I commented out the checks for the two behaviors you pointed
> > > out.
> > > 
> > > Can I get a review so we can push this in and move to the next step ?

> On Sun, 2018-04-22 at 09:25 +1200, Gary Lockyer via samba-technical
> wrote:

> > Minor nit-pick isn't the max line length 80 characters.  

> You are right, attached fixed patch.

A couple more comments, now that I've tested it on more changes...  :-)

* It doesn't like comments like this:

  /*
     Hello world
  */

  Either do I!  :-)  However, README.Coding doesn't say anything about
  this and we have plenty of them in the code.  Not sure what to
  suggest here.

* The "Un-cuddled open brace" thing is going to be annoying

  If you're initialising an array of structures then I don't see how
  you can avoid this warning.

  Similarly, when initialising a fairly long array of structures you
  might want to have the values inline, with opening and closing braces
  on the same line as the data.  However, this generates lots of
  complaints.  :-(

* "Preprocessor statement in function body" is also interesting

  We have:

static bool syslog_log_validate(const char *option)
{
        if (option == NULL) {
                return true;
#ifdef _PATH_LOG
        } else if (strcmp(option, "nonblocking") == 0) {
                return true;
#endif
        } else if (strcmp(option, "udp") == 0) {
                return true;
        } else if (strcmp(option, "udp-rfc5424") == 0) {
                return true;
        }

        return false;
}

  This seems like a sensible thing to do...

peace & happiness,
martin



More information about the samba-technical mailing list