Re: [PATCH 3/7] Fix inconsistency in using curly braces around functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 17, 2014 at 09:25:13AM -0600, Eric Blake wrote:
> On 03/17/2014 08:39 AM, Martin Kletzander wrote:
> > Although not explicitly requested, we are using K&R (or Kernel)
> > indentation for curly braces around functions in HACKING file and most
> > of the code.  The rest is inconsistent and this patch is trying to fix
> > the most of it.
> > 
> > Found by:
> > 
> > git grep -nH -e '^\s*\*\?[_a-zA-Z0-9]\+\(,\? \*\?[_a-zA-Z0-9]\+\)\+) \?{$' \
> >  -e '^\s*[_a-zA-Z0-9]\+\( [_a-zA-Z0-9]\+\)*(\*\?[_a-zA-Z0-9]\+\(,\? \*\?[_a-zA-Z0-9]\+\)\+) \?{$
> > 
> > and skipped foreach constructs which were found as well.
> > 
> > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> > ---
> 
> This one's big.  I'm reluctant to ack as-is; I think it could use two
> things: first, can you split it into a series of smaller patches
> (convert one directory or so at a time); second, add a cfg.mk check to
> enforce the style, so outliers don't sneak back in.
> 

I'll send a v2 split into smaller patches and I'll wrap long lines
too, no problem with that, but...

I was trying to tune the regexp to achieve 0 false positives and we
would need to use PCRE regexp which I don't know whether it's
supported by the sc_prohibit_ syntax checks.

The resulting regexps (or rather the whole command) look like this
(sorry for the long line):

git grep -nHP -e '^\s*\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9]+)+\) ?\{' -e
'^\s*(?!([a-zA-Z_]*for_?each[a-zA-Z_]*) ?\()[_a-zA-Z0-9]+( [_a-zA-Z0-9]+)* ?\(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+\) ?\{'

or (listing all allowed foreach methods:

git grep -nHP -e '^\s*\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9]+)+\) ?\{' -e
'^\s*(?!(libxl_for_each_set_bit|udev_list_entry_foreach|nla_for_each_nested) ?\()[_a-zA-Z0-9]+( [_a-zA-Z0-9]+)* ?\(\*?[_a-zA-Z0-9]+(,? \*?[_a-zA-Z0-9\[\]]+)+\) ?\{'

Let me know if we can somehow incorporate it inside cfg.mk, I'll
gladly do that.  If not, bracket-spacing.pl will probably do the
trick, but anyway, we have to filter to *.[hc] files only.

Martin

> 
> > +++ b/daemon/libvirtd-config.c
> 
> > @@ -156,7 +156,8 @@ checkType(virConfValuePtr p, const char *filename,
> >      } while (0)
> > 
> > 
> > -static int remoteConfigGetAuth(virConfPtr conf, const char *key, int *auth, const char *filename) {
> > +static int remoteConfigGetAuth(virConfPtr conf, const char *key, int *auth, const char *filename)
> 
> Hmm, this line is still longer than 80 columns.  While touching this,
> should we also do:
> 
> static int
> remoteConfigGetAuth(virConfPtr conf,
>                     const char *key,
>                     int *auth,
>                     const char *filename)
> 
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]