[PATCH] Add utilities to check style

[Simo]

On Mon, 2018-04-23 at 16:13 +1000, Martin Schwenke wrote:
> 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.

Unless we have a strong desire to allow those, I would just switch to
the more consistent checks this tool performs.
More checks are not necessarily bad unless we have an explicit reason
to dislike a more specific style check.
More consistency is always a good thing in my book, more freedom is ok
only when there is a good reason to allow it.

> * 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.  :-(

I do not see why we want this to be a thing to be honest,.

We do:
  struct foo bar = {
    data
  }
We do not do:
  struct foo bar = 
  {
     data
  }
do we ?

> * "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...

Tend do be horrible for readbility and also source of code flow errors,
but if we *really* want that I guess we can disable that check too.

> peace & happiness,
> martin


Attached new patch that disables also preprocessor statement checks,
but keeps the rest. including un-cuddled errors. Can you show me an
example where the style using a bare braces is desirable ?

Simo.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-utilities-to-check-style.patch
Type: text/x-patch
Size: 20902 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180423/9480be95/0001-Add-utilities-to-check-style.bin>