On Fri, Nov 15, 2019 at 04:50:57PM -0500, Cole Robinson wrote: > On 11/11/19 9:38 AM, Daniel P. Berrangé wrote: > > As part of an goal to eliminate Perl from libvirt build tools, > > rewrite the check-drivername.pl tool in Python. > > > > This was mostly a straight conversion, manually going line-by-line > > to change the syntax from Perl to Python. Thus the overall structure > > of the file and approach is the same. > > > > In testing though it was discovered the existing code was broken > > since it hadn't been updated after driver.h was split into many > > files. Since the old code is being thrown away, the fix was done > > as part of the rewrite rather than split into a separate commit. > > > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > --- > > Makefile.am | 1 + > > scripts/check-drivername.py | 114 ++++++++++++++++++++++++++++++++++++ > > src/Makefile.am | 18 ++++-- > > src/check-drivername.pl | 83 -------------------------- > > 4 files changed, 129 insertions(+), 87 deletions(-) > > create mode 100644 scripts/check-drivername.py > > delete mode 100755 src/check-drivername.pl > > > > diff --git a/Makefile.am b/Makefile.am > > index afed409c94..7155ab6ce9 100644 > > --- a/Makefile.am > > +++ b/Makefile.am > > @@ -47,6 +47,7 @@ EXTRA_DIST = \ > > AUTHORS.in \ > > scripts/augeas-gentest.py \ > > scripts/check-aclperms.py \ > > + scripts/check-drivername.py \ > > scripts/check-spacing.py \ > > scripts/check-symfile.py \ > > scripts/check-symsorting.py \ > > diff --git a/scripts/check-drivername.py b/scripts/check-drivername.py > > new file mode 100644 > > index 0000000000..3226ee7962 > > --- /dev/null > > +++ b/scripts/check-drivername.py > > @@ -0,0 +1,114 @@ > > +#!/usr/bin/env python > > +# > > +# Copyright (C) 2013-2019 Red Hat, Inc. > > +# > > +# This library is free software; you can redistribute it and/or > > +# modify it under the terms of the GNU Lesser General Public > > +# License as published by the Free Software Foundation; either > > +# version 2.1 of the License, or (at your option) any later version. > > +# > > +# This library is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > +# Lesser General Public License for more details. > > +# > > +# You should have received a copy of the GNU Lesser General Public > > +# License along with this library. If not, see > > +# <http://www.gnu.org/licenses/>. > > +# > > + > > +from __future__ import print_function > > + > > +import re > > +import sys > > + > > +drvfiles = [] > > +symfiles = [] > > +for arg in sys.argv: > > + if arg.endswith(".h"): > > + drvfiles.append(arg) > > + else: > > + symfiles.append(arg) > > + > > +symbols = {} > > + > > +for symfile in symfiles: > > + with open(symfile, "r") as fh: > > + for line in fh: > > + m = re.search(r'''^\s*(vir\w+)\s*;\s*$''', line) > > + if m is not None: > > + symbols[m.group(1)] = True > > + > > +status = 0 > > +for drvfile in drvfiles: > > + with open(drvfile, "r") as fh: > > + for line in fh: > > + if line.find("virDrvConnectSupportsFeature") != -1: > > + continue > > + > > I think this bit is just another mechanism for the skip list below. I > can't really figure out what other purpose it could be serving Yes, this is just an artifact of the original code. It can be merged below. > > > > > + m = re.search(r'''\*(virDrv\w+)\s*\)''', line) > > + if m is not None: > > + drv = m.group(1) > > + > > + skip = [ > > + "virDrvStateInitialize", > > + "virDrvStateCleanup", > > + "virDrvStateReload", > > + "virDrvStateStop", > > + "virDrvConnectURIProbe", > > + "virDrvDomainMigratePrepare", > > + "virDrvDomainMigratePrepare2", > > + "virDrvDomainMigratePrepare3", > > + "virDrvDomainMigratePrepare3Params", > > + "virDrvDomainMigratePrepareTunnel", > > + "virDrvDomainMigratePrepareTunnelParams", > > + "virDrvDomainMigratePrepareTunnel3", > > + "virDrvDomainMigratePrepareTunnel3Params", > > + "virDrvDomainMigratePerform", > > + "virDrvDomainMigratePerform3", > > + "virDrvDomainMigratePerform3Params", > > + "virDrvDomainMigrateConfirm", > > + "virDrvDomainMigrateConfirm3", > > + "virDrvDomainMigrateConfirm3Params", > > + "virDrvDomainMigrateBegin", > > + "virDrvDomainMigrateBegin3", > > + "virDrvDomainMigrateBegin3Params", > > + "virDrvDomainMigrateFinish", > > + "virDrvDomainMigrateFinish2", > > + "virDrvDomainMigrateFinish3", > > + "virDrvDomainMigrateFinish3Params", > > + "virDrvStreamInData", > > + ] > > + if drv in skip: > > + continue > > + > > + sym = drv.replace("virDrv", "vir") > > + > > + if sym not in symbols: > > + print("Driver method name %s doesn't match public API" % > > + drv) > > Missing status = 1 here > > > + continue > > + > > + m = re.search(r'''^\*(vir\w+)\s*\)''', line) > > + if m is not None: > > + name = m.group(1) > > + print("Bogus name %s" % name) > > + status = 1 > > + continue > > + > > Not quite sure what this condition is supposed to be catching. It's > trying to match lines that start with '*vir'. I guess it's trying to > warning against anything that doesn't start with 'virDrv' and instead > plain 'vir', but the anchor usage is messed up. But it looks > pre-existing. Anyways, the two other error messages trigger correctly It matches if you define an API without the 'Drv' prefix. I just need to drop the "^" eg if i did diff --git a/src/driver-secret.h b/src/driver-secret.h index 125238fe7b..ee93eaba68 100644 --- a/src/driver-secret.h +++ b/src/driver-secret.h @@ -35,7 +35,7 @@ typedef virSecretPtr const unsigned char *uuid); typedef virSecretPtr -(*virDrvSecretLookupByUsage)(virConnectPtr conn, +(*virFishSecretLookupByUsage)(virConnectPtr conn, int usageType, const char *usageID); then it would report Bogus name *virFishSecretLookupByUsage Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
