[libvirt] [PATCH] maint: fix syntax-check sc_prohibit_int_ijk exclude rule

Pavel Hrdina phrdina at redhat.com
Tue May 24 11:24:11 UTC 2016


On Tue, May 24, 2016 at 11:42:11AM +0200, Andrea Bolognani wrote:
> On Tue, 2016-05-24 at 09:41 +0200, Pavel Hrdina wrote:
> > This fixes the "include/" path, we need to create a regex for the whole path
> > because we are matching the whole line.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > 
> > I'm surprised that it even worked so far.  I've noticed this after system update
> > few days ago.
> 
> So, is syntax-check failing for you without that patch? Because
> it's working for me.

Yes it's failing right now for me, like i wrote as note it started after system
updated.

> I think it worked so far because the line ends with '$' instead
> of '$$' - I'm not sure what a single '$' does in this context,
> but it definitely doesn't force the regex to match the whole path.

That's probably why it started failing for me, because it started parsing the
single '$'.

> 
> >  cfg.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/cfg.mk b/cfg.mk
> > index c19f615..f688cbb 100644
> > --- a/cfg.mk
> > +++ b/cfg.mk
> > @@ -1259,7 +1259,7 @@ exclude_file_name_regexp--sc_prohibit_include_public_headers_brackets = \
> >    ^(tools/|examples/|include/libvirt/(virterror|libvirt(-(admin|qemu|lxc))?)\.h$$)
> >  
> >  exclude_file_name_regexp--sc_prohibit_int_ijk = \
> > -  ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/|src/admin_protocol-
> > structs|src/admin/admin_protocol.x)$
> > +  ^(src/remote_protocol-structs|src/remote/remote_protocol.x|cfg.mk|include/libvirt/libvirt.+|src/admin_protocol-
> > structs|src/admin/admin_protocol.x)$$
>> >  exclude_file_name_regexp--sc_prohibit_getenv = \
> >    ^tests/.*\.[ch]$$
> 
> ACK to the change.

Thanks

> You should also, as follow-up fixes, properly escape dots and
> maybe even use a regex to match the other files instead of
> listing them: the end result would look something like
> 
> ^(cfg\.mk|include/libvirt/libvirt.+|src/(admin|remote)_protocol-structs|src/(admin|remote)/(admin|remote)_protocol\.x)$$

Escaping dots is a good idea and it should be done, I'll include it in this
patch, it's probably not worth to do in separate commit.  I'll explain the
changes little bit more in the commit message.

> We might go even farther and turn it into
> 
> ^(cfg\.mk|include/libvirt/libvirt.+|src/.+_protocol-structs|src/.+/.+_protocol\.x)$$

but this is probably intentional to exclude only the files we know about so if
someone else update a different file the syntax-check will fail.

Pavel




More information about the libvir-list mailing list