[libvirt] [PATCHv2 00/25] Python tweaks

Changes since v1 (https://www.redhat.com/archives/libvir-list/2018-March/msg01001.html) 01 - Fixed typo. 03 - Added whitespace after comma in: value = value + re.sub("^(\d+)U$","\\1", token[1]) 05 and 06 - Use printf style format and break strings everywhere there is a newline. 07 - Use try-except instead of an explicit check. 10 - Use isalnum() instead of a regex match, and a few more simplifications were added. 20 - Use try-except instead of an explicit check. 23 - Don't slice `line` in the loop condition. New patches: 9, 24, 25 The patches have been tested with `make check`, `make syntax-check`. The output of apibuild.py was tested with `cd docs/` followed by: srcdir=. builddir=. /usr/bin/python2 ./apibuild.py; sha1sum *.xml | sha1sum and srcdir=. builddir=. /usr/bin/python3 ./apibuild.py; sha1sum *.xml | sha1sum before and after the patches were applied. Radostin Stoyanov (25): apibuild: Use isinstance for type checking apibuild: Split imports on separate lines apibuild: Remove whitespace before ',' and ':' python: Add whitespace around = and % operators esx_vi_generator: Simplify generate_helper_source esx_vi_generator: Simplify generate_helper_header esx_vi_generator: Simplify get_occurrence_comment esx_vi_generator: Simplify alignment function cfg.mk: check ctype_macros only on *.[c|h] files apibuild: Simplify conditional statements python: Remove space around = in keyword args WmiClass: Don't share "versions" between instances apibuild: Simplify uniq function apibuild: Avoid double sorting of ids python3: cpu-reformat: Use the print() function apibuild: Drop backslash between brackets apibuild: Fix indentation not multiple of 4 apibuild: Simplify strip_lead_star() apibuild: Simplify parseTypeComment() apibuild: Simplify type checking of literals apibuild: Use list comprehension insteand of map apibuild: Simplify merging of preproc tokens apibuild: Simplify parsing string tokens apibuild: Remove redundant parentheses apibuild: Simplify getline() cfg.mk | 1 + docs/apibuild.py | 734 +++++++++++++++++-------------------- docs/index.py | 54 +-- src/esx/esx_vi_generator.py | 183 ++++----- src/hyperv/hyperv_wmi_generator.py | 36 +- tests/cputestdata/cpu-cpuid.py | 6 +- tests/cputestdata/cpu-reformat.py | 6 +- 7 files changed, 472 insertions(+), 548 deletions(-) -- 2.14.3

The isinstance() function [1] returns true if an object argument is an instance of a classinfo argument or of a direct, indirect subclass thereof. 1: https://docs.python.org/3/library/functions.html#isinstance Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 17d14a0c5..832f04ab1 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -742,7 +742,7 @@ class CParser: return line def cleanupComment(self): - if type(self.comment) != type(""): + if not isinstance(self.comment, str): return # remove the leading * on multi-line comments lines = self.comment.splitlines(True) @@ -2223,9 +2223,8 @@ class docBuilder: output.write(" <struct name='%s' file='%s' type='%s'" % ( name, self.modulename_file(id.header), id.info)) name = id.info[7:] - if name in self.idx.structs and ( \ - type(self.idx.structs[name].info) == type(()) or - type(self.idx.structs[name].info) == type([])): + if (name in self.idx.structs and + isinstance(self.idx.structs[name].info, (list, tuple))): output.write(">\n") try: for field in self.idx.structs[name].info: -- 2.14.3

PEP8 recommends imports to be on separate lines. [1] 1: https://www.python.org/dev/peps/pep-0008/#imports Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 832f04ab1..7507c1d26 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -10,7 +10,8 @@ from __future__ import print_function -import os, sys +import os +import sys import glob import re -- 2.14.3

PEP8 recommends removing whitespace immediately before a comma, semicolon, or colon [1]. In addition remove multiple spaces after keyword (PEP8 - E271). 1: https://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-stat... Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 104 +++++++++++++++++++------------------ docs/index.py | 22 ++++---- src/esx/esx_vi_generator.py | 102 +++++++++++++++++------------------- src/hyperv/hyperv_wmi_generator.py | 32 ++++++------ 4 files changed, 128 insertions(+), 132 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 7507c1d26..73a7535db 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -590,7 +590,7 @@ class CLexer: # line[i] == '>' or line[i] == '<' or line[i] == '=' or \ # line[i] == '/' or line[i] == '%' or line[i] == '&' or \ # line[i] == '!' or line[i] == '|' or line[i] == '.': - if line[i] == '.' and i + 2 < l and \ + if line[i] == '.' and i + 2 < l and \ line[i+1] == '.' and line[i+2] == '.': self.tokens.append(('name', '...')) i = i + 3 @@ -1401,7 +1401,7 @@ class CParser: while token[0] != "sep" or (token[1] != ',' and token[1] != '}'): # We might be dealing with '1U << 12' here - value = value + re.sub("^(\d+)U$","\\1", token[1]) + value = value + re.sub("^(\d+)U$", "\\1", token[1]) token = self.token() else: try: @@ -1735,7 +1735,7 @@ class CParser: while token is not None and token[0] == "op" and token[1] == '*': self.type = self.type + token[1] token = self.token() - if token is None or token[0] != "name" : + if token is None or token[0] != "name": self.error("parsing function type, name expected", token) return token self.type = self.type + token[1] @@ -1825,41 +1825,42 @@ class CParser: # this dict contains the functions that are allowed to use [unsigned] # long for legacy reasons in their signature and return type. this list is # fixed. new procedures and public APIs have to use [unsigned] long long - long_legacy_functions = \ - { "virGetVersion" : (False, ("libVer", "typeVer")), - "virConnectGetLibVersion" : (False, ("libVer")), - "virConnectGetVersion" : (False, ("hvVer")), - "virDomainGetMaxMemory" : (True, ()), - "virDomainMigrate" : (False, ("flags", "bandwidth")), - "virDomainMigrate2" : (False, ("flags", "bandwidth")), - "virDomainMigrateBegin3" : (False, ("flags", "bandwidth")), - "virDomainMigrateConfirm3" : (False, ("flags", "bandwidth")), - "virDomainMigrateDirect" : (False, ("flags", "bandwidth")), - "virDomainMigrateFinish" : (False, ("flags")), - "virDomainMigrateFinish2" : (False, ("flags")), - "virDomainMigrateFinish3" : (False, ("flags")), - "virDomainMigratePeer2Peer" : (False, ("flags", "bandwidth")), - "virDomainMigratePerform" : (False, ("flags", "bandwidth")), - "virDomainMigratePerform3" : (False, ("flags", "bandwidth")), - "virDomainMigratePrepare" : (False, ("flags", "bandwidth")), - "virDomainMigratePrepare2" : (False, ("flags", "bandwidth")), - "virDomainMigratePrepare3" : (False, ("flags", "bandwidth")), - "virDomainMigratePrepareTunnel" : (False, ("flags", "bandwidth")), - "virDomainMigratePrepareTunnel3" : (False, ("flags", "bandwidth")), - "virDomainMigrateToURI" : (False, ("flags", "bandwidth")), - "virDomainMigrateToURI2" : (False, ("flags", "bandwidth")), - "virDomainMigrateVersion1" : (False, ("flags", "bandwidth")), - "virDomainMigrateVersion2" : (False, ("flags", "bandwidth")), - "virDomainMigrateVersion3" : (False, ("flags", "bandwidth")), - "virDomainMigrateSetMaxSpeed" : (False, ("bandwidth")), - "virDomainSetMaxMemory" : (False, ("memory")), - "virDomainSetMemory" : (False, ("memory")), - "virDomainSetMemoryFlags" : (False, ("memory")), - "virDomainBlockCommit" : (False, ("bandwidth")), - "virDomainBlockJobSetSpeed" : (False, ("bandwidth")), - "virDomainBlockPull" : (False, ("bandwidth")), - "virDomainBlockRebase" : (False, ("bandwidth")), - "virDomainMigrateGetMaxSpeed" : (False, ("bandwidth")) } + long_legacy_functions = { + "virGetVersion": (False, ("libVer", "typeVer")), + "virConnectGetLibVersion": (False, ("libVer")), + "virConnectGetVersion": (False, ("hvVer")), + "virDomainGetMaxMemory": (True, ()), + "virDomainMigrate": (False, ("flags", "bandwidth")), + "virDomainMigrate2": (False, ("flags", "bandwidth")), + "virDomainMigrateBegin3": (False, ("flags", "bandwidth")), + "virDomainMigrateConfirm3": (False, ("flags", "bandwidth")), + "virDomainMigrateDirect": (False, ("flags", "bandwidth")), + "virDomainMigrateFinish": (False, ("flags")), + "virDomainMigrateFinish2": (False, ("flags")), + "virDomainMigrateFinish3": (False, ("flags")), + "virDomainMigratePeer2Peer": (False, ("flags", "bandwidth")), + "virDomainMigratePerform": (False, ("flags", "bandwidth")), + "virDomainMigratePerform3": (False, ("flags", "bandwidth")), + "virDomainMigratePrepare": (False, ("flags", "bandwidth")), + "virDomainMigratePrepare2": (False, ("flags", "bandwidth")), + "virDomainMigratePrepare3": (False, ("flags", "bandwidth")), + "virDomainMigratePrepareTunnel": (False, ("flags", "bandwidth")), + "virDomainMigratePrepareTunnel3": (False, ("flags", "bandwidth")), + "virDomainMigrateToURI": (False, ("flags", "bandwidth")), + "virDomainMigrateToURI2": (False, ("flags", "bandwidth")), + "virDomainMigrateVersion1": (False, ("flags", "bandwidth")), + "virDomainMigrateVersion2": (False, ("flags", "bandwidth")), + "virDomainMigrateVersion3": (False, ("flags", "bandwidth")), + "virDomainMigrateSetMaxSpeed": (False, ("bandwidth")), + "virDomainSetMaxMemory": (False, ("memory")), + "virDomainSetMemory": (False, ("memory")), + "virDomainSetMemoryFlags": (False, ("memory")), + "virDomainBlockCommit": (False, ("bandwidth")), + "virDomainBlockJobSetSpeed": (False, ("bandwidth")), + "virDomainBlockPull": (False, ("bandwidth")), + "virDomainBlockRebase": (False, ("bandwidth")), + "virDomainMigrateGetMaxSpeed": (False, ("bandwidth")) + } def checkLongLegacyFunction(self, name, return_type, signature): if "long" in return_type and "long long" not in return_type: @@ -1883,10 +1884,11 @@ class CParser: # this dict contains the structs that are allowed to use [unsigned] # long for legacy reasons. this list is fixed. new structs have to use # [unsigned] long long - long_legacy_struct_fields = \ - { "_virDomainInfo" : ("maxMem", "memory"), - "_virNodeInfo" : ("memory"), - "_virDomainBlockJobInfo" : ("bandwidth") } + long_legacy_struct_fields = { + "_virDomainInfo": ("maxMem", "memory"), + "_virNodeInfo": ("memory"), + "_virDomainBlockJobInfo": ("bandwidth") + } def checkLongLegacyStruct(self, name, fields): for field in fields: @@ -1934,7 +1936,7 @@ class CParser: elif token[1] == 'static': static = 1 token = self.token() - if token is None or token[0] != 'name': + if token is None or token[0] != 'name': return token if token[1] == 'typedef': @@ -2205,7 +2207,7 @@ class docBuilder: output.write(" </macro>\n") def serialize_union(self, output, field, desc): - output.write(" <field name='%s' type='union' info='%s'>\n" % (field[1] , desc)) + output.write(" <field name='%s' type='union' info='%s'>\n" % (field[1], desc)) output.write(" <union>\n") for f in field[3]: desc = f[2] @@ -2213,7 +2215,7 @@ class docBuilder: desc = '' else: desc = escape(desc) - output.write(" <field name='%s' type='%s' info='%s'/>\n" % (f[1] , f[0], desc)) + output.write(" <field name='%s' type='%s' info='%s'/>\n" % (f[1], f[0], desc)) output.write(" </union>\n") output.write(" </field>\n") @@ -2238,13 +2240,13 @@ class docBuilder: if field[0] == "union": self.serialize_union(output, field, desc) else: - output.write(" <field name='%s' type='%s' info='%s'/>\n" % (field[1] , field[0], desc)) + output.write(" <field name='%s' type='%s' info='%s'/>\n" % (field[1], field[0], desc)) except: self.warning("Failed to serialize struct %s" % (name)) output.write(" </struct>\n") else: output.write("/>\n") - else : + else: output.write(" <typedef name='%s' file='%s' type='%s'" % ( name, self.modulename_file(id.header), id.info)) try: @@ -2401,7 +2403,7 @@ class docBuilder: typ = sorted(funcs.keys()) for type in typ: if type == '' or type == 'void' or type == "int" or \ - type == "char *" or type == "const char *" : + type == "char *" or type == "const char *": continue output.write(" <type name='%s'>\n" % (type)) ids = funcs[type] @@ -2430,7 +2432,7 @@ class docBuilder: typ = sorted(funcs.keys()) for type in typ: if type == '' or type == 'void' or type == "int" or \ - type == "char *" or type == "const char *" : + type == "char *" or type == "const char *": continue output.write(" <type name='%s'>\n" % (type)) ids = sorted(funcs[type]) @@ -2589,7 +2591,7 @@ class app: builddir = os.path.abspath((os.environ["builddir"])) if srcdir == builddir: builddir = None - if glob.glob(srcdir + "/../src/libvirt.c") != [] : + if glob.glob(srcdir + "/../src/libvirt.c") != []: if not quiet: print("Rebuilding API description for %s" % name) dirs = [srcdir + "/../src", @@ -2599,7 +2601,7 @@ class app: not os.path.exists(srcdir + "/../include/libvirt/libvirt-common.h")): dirs.append(builddir + "/../include/libvirt") builder = docBuilder(name, srcdir, dirs, []) - elif glob.glob("src/libvirt.c") != [] : + elif glob.glob("src/libvirt.c") != []: if not quiet: print("Rebuilding API description for %s" % name) builder = docBuilder(name, srcdir, diff --git a/docs/index.py b/docs/index.py index bedec8ae0..e2f9185c3 100755 --- a/docs/index.py +++ b/docs/index.py @@ -59,21 +59,21 @@ libxml2.registerErrorHandler(callback, None) # to create them # TABLES={ - "symbols" : """CREATE TABLE symbols ( + "symbols": """CREATE TABLE symbols ( name varchar(255) BINARY NOT NULL, module varchar(255) BINARY NOT NULL, type varchar(25) NOT NULL, descr varchar(255), UNIQUE KEY name (name), KEY module (module))""", - "words" : """CREATE TABLE words ( + "words": """CREATE TABLE words ( name varchar(50) BINARY NOT NULL, symbol varchar(255) BINARY NOT NULL, relevance int, KEY name (name), KEY symbol (symbol), UNIQUE KEY ID (name, symbol))""", - "wordsHTML" : """CREATE TABLE wordsHTML ( + "wordsHTML": """CREATE TABLE wordsHTML ( name varchar(50) BINARY NOT NULL, resource varchar(255) BINARY NOT NULL, section varchar(255), @@ -82,30 +82,30 @@ TABLES={ KEY name (name), KEY resource (resource), UNIQUE KEY ref (name, resource))""", - "wordsArchive" : """CREATE TABLE wordsArchive ( + "wordsArchive": """CREATE TABLE wordsArchive ( name varchar(50) BINARY NOT NULL, ID int(11) NOT NULL, relevance int, KEY name (name), UNIQUE KEY ref (name, ID))""", - "pages" : """CREATE TABLE pages ( + "pages": """CREATE TABLE pages ( resource varchar(255) BINARY NOT NULL, title varchar(255) BINARY NOT NULL, UNIQUE KEY name (resource))""", - "archives" : """CREATE TABLE archives ( + "archives": """CREATE TABLE archives ( ID int(11) NOT NULL auto_increment, resource varchar(255) BINARY NOT NULL, title varchar(255) BINARY NOT NULL, UNIQUE KEY id (ID,resource(255)), INDEX (ID), INDEX (resource))""", - "Queries" : """CREATE TABLE Queries ( + "Queries": """CREATE TABLE Queries ( ID int(11) NOT NULL auto_increment, Value varchar(50) NOT NULL, Count int(11) NOT NULL, UNIQUE KEY id (ID,Value(35)), INDEX (ID))""", - "AllQueries" : """CREATE TABLE AllQueries ( + "AllQueries": """CREATE TABLE AllQueries ( ID int(11) NOT NULL auto_increment, Value varchar(50) NOT NULL, Count int(11) NOT NULL, @@ -171,7 +171,7 @@ def checkTables(db, verbose = 1): if verbose: print "Table %s contains %d records" % (table, row[0]) except: - print "Troubles with table %s : repairing" % (table) + print "Troubles with table %s: repairing" % (table) ret = c.execute("repair table %s" % table) print "repairing returned %d" % (ret) ret = c.execute("SELECT count(*) from %s" % table) @@ -1041,7 +1041,7 @@ def analyzeHTMLPages(): doc = libxml2.htmlParseFile(html, None) try: res = analyzeHTML(doc, html) - print "Parsed %s : %d paragraphs" % (html, res) + print "Parsed %s: %d paragraphs" % (html, res) ret = ret + 1 except: print "could not parse %s" % (html) @@ -1230,7 +1230,7 @@ def main(): elif args[i] == '--archive-year': i = i + 1 year = args[i] - months = ["January" , "February", "March", "April", "May", + months = ["January", "February", "March", "April", "May", "June", "July", "August", "September", "October", "November", "December"] for month in months: diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 545f8bdda..17ac43cb7 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1342,61 +1342,53 @@ predefined_objects = ["AnyType", "MethodFault", "ManagedObjectReference"] -additional_enum_features = { "ManagedEntityStatus" : Enum.FEATURE__ANY_TYPE, - "TaskInfoState" : Enum.FEATURE__ANY_TYPE, - "VirtualMachinePowerState" : Enum.FEATURE__ANY_TYPE } - -additional_object_features = { "AutoStartDefaults" : Object.FEATURE__ANY_TYPE, - "AutoStartPowerInfo" : Object.FEATURE__ANY_TYPE, - "DatastoreHostMount" : Object.FEATURE__DEEP_COPY | - Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "DatastoreInfo" : Object.FEATURE__ANY_TYPE | - Object.FEATURE__DYNAMIC_CAST, - "HostConfigManager" : Object.FEATURE__ANY_TYPE, - "HostCpuIdInfo" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "HostDatastoreBrowserSearchResults" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "HostHostBusAdapter" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "HostInternetScsiHba" : Object.FEATURE__DYNAMIC_CAST | - Object.FEATURE__DEEP_COPY, - "HostInternetScsiTargetTransport" : Object.FEATURE__DYNAMIC_CAST, - "HostScsiDisk" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE | - Object.FEATURE__DYNAMIC_CAST, - "HostScsiTopologyInterface" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "HostScsiTopologyLun" : Object.FEATURE__ANY_TYPE | - Object.FEATURE__LIST | - Object.FEATURE__DEEP_COPY, - "HostScsiTopologyTarget" : Object.FEATURE__ANY_TYPE | - Object.FEATURE__LIST, - "HostPortGroup" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "HostVirtualSwitch" : Object.FEATURE__DEEP_COPY | - Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "ManagedObjectReference" : Object.FEATURE__ANY_TYPE, - "ObjectContent" : Object.FEATURE__DEEP_COPY, - "PhysicalNic" : Object.FEATURE__DEEP_COPY | - Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "ResourcePoolResourceUsage" : Object.FEATURE__ANY_TYPE, - "ScsiLun" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE | - Object.FEATURE__DEEP_COPY, - "ScsiLunDurableName" : Object.FEATURE__LIST, - "ServiceContent" : Object.FEATURE__DESERIALIZE, - "SharesInfo" : Object.FEATURE__ANY_TYPE, - "TaskInfo" : Object.FEATURE__LIST | - Object.FEATURE__ANY_TYPE, - "UserSession" : Object.FEATURE__ANY_TYPE, - "VirtualMachineQuestionInfo" : Object.FEATURE__ANY_TYPE, - "VirtualMachineSnapshotTree" : Object.FEATURE__DEEP_COPY | - Object.FEATURE__ANY_TYPE, - "VmEventArgument" : Object.FEATURE__DESERIALIZE } +additional_enum_features = { + "ManagedEntityStatus": Enum.FEATURE__ANY_TYPE, + "TaskInfoState": Enum.FEATURE__ANY_TYPE, + "VirtualMachinePowerState": Enum.FEATURE__ANY_TYPE +} + +additional_object_features = { + "AutoStartDefaults": Object.FEATURE__ANY_TYPE, + "AutoStartPowerInfo": Object.FEATURE__ANY_TYPE, + "DatastoreHostMount": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE), + "DatastoreInfo": Object.FEATURE__ANY_TYPE | Object.FEATURE__DYNAMIC_CAST, + "HostConfigManager": Object.FEATURE__ANY_TYPE, + "HostCpuIdInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, + "HostDatastoreBrowserSearchResults": (Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE), + "HostHostBusAdapter": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, + "HostInternetScsiHba": (Object.FEATURE__DYNAMIC_CAST | + Object.FEATURE__DEEP_COPY), + "HostInternetScsiTargetTransport": Object.FEATURE__DYNAMIC_CAST, + "HostScsiDisk": (Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE | + Object.FEATURE__DYNAMIC_CAST), + "HostScsiTopologyInterface": (Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE), + "HostScsiTopologyLun": (Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST | + Object.FEATURE__DEEP_COPY), + "HostScsiTopologyTarget": Object.FEATURE__ANY_TYPE | Object.FEATURE__LIST, + "HostPortGroup": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, + "HostVirtualSwitch": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE), + "ManagedObjectReference": Object.FEATURE__ANY_TYPE, + "ObjectContent": Object.FEATURE__DEEP_COPY, + "PhysicalNic": (Object.FEATURE__DEEP_COPY | Object.FEATURE__LIST | + Object.FEATURE__ANY_TYPE), + "ResourcePoolResourceUsage": Object.FEATURE__ANY_TYPE, + "ScsiLun": (Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE | + Object.FEATURE__DEEP_COPY), + "ScsiLunDurableName": Object.FEATURE__LIST, + "ServiceContent": Object.FEATURE__DESERIALIZE, + "SharesInfo": Object.FEATURE__ANY_TYPE, + "TaskInfo": Object.FEATURE__LIST | Object.FEATURE__ANY_TYPE, + "UserSession": Object.FEATURE__ANY_TYPE, + "VirtualMachineQuestionInfo": Object.FEATURE__ANY_TYPE, + "VirtualMachineSnapshotTree": (Object.FEATURE__DEEP_COPY | + Object.FEATURE__ANY_TYPE), + "VmEventArgument": Object.FEATURE__DESERIALIZE +} removed_object_features = {} diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py index 9e7ea4382..6e98f0a89 100755 --- a/src/hyperv/hyperv_wmi_generator.py +++ b/src/hyperv/hyperv_wmi_generator.py @@ -342,21 +342,23 @@ class WmiClassVersion: class Property: - typemap = {"boolean" : "BOOL", - "string" : "STR", - "datetime" : "STR", - "int8" : "INT8", - "sint8" : "INT8", - "int16" : "INT16", - "sint16" : "INT16", - "int32" : "INT32", - "sint32" : "INT32", - "int64" : "INT64", - "sint64" : "INT64", - "uint8" : "UINT8", - "uint16" : "UINT16", - "uint32" : "UINT32", - "uint64" : "UINT64"} + typemap = { + "boolean": "BOOL", + "string": "STR", + "datetime": "STR", + "int8": "INT8", + "sint8": "INT8", + "int16": "INT16", + "sint16": "INT16", + "int32": "INT32", + "sint32": "INT32", + "int64": "INT64", + "sint64": "INT64", + "uint8": "UINT8", + "uint16": "UINT16", + "uint32": "UINT32", + "uint64": "UINT64" + } def __init__(self, type, name, is_array): -- 2.14.3

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 18 +++++++++--------- docs/index.py | 6 +++--- tests/cputestdata/cpu-cpuid.py | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 73a7535db..a11c2d145 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -15,10 +15,10 @@ import sys import glob import re -quiet=True -warnings=0 -debug=False -debugsym=None +quiet = True +warnings = 0 +debug = False +debugsym = None # # C parser analysis code @@ -132,7 +132,7 @@ def escape(raw): def uniq(items): d = {} for item in items: - d[item]=1 + d[item] = 1 k = sorted(d.keys()) return k @@ -1408,7 +1408,7 @@ class CParser: value = "%d" % (int(value) + 1) except: self.warning("Failed to compute value of enum %s" % (name)) - value="" + value = "" if token[0] == "sep" and token[1] == ",": if commentsBeforeVal: self.cleanupComment() @@ -2286,7 +2286,7 @@ class docBuilder: if apstr != "": apstr = apstr + " && " apstr = apstr + cond - output.write(" <cond>%s</cond>\n"% (apstr)) + output.write(" <cond>%s</cond>\n" % (apstr)) try: (ret, params, desc) = id.info output.write(" <info><![CDATA[%s]]></info>\n" % (desc)) @@ -2479,7 +2479,7 @@ class docBuilder: output.write(" </letter>\n") output.write(" </chunk>\n") count = 0 - chunks.append(["chunk%s" % (chunk -1), first_letter, letter]) + chunks.append(["chunk%s" % (chunk - 1), first_letter, letter]) output.write(" <chunk name='chunk%s'>\n" % (chunk)) first_letter = id[0] chunk = chunk + 1 @@ -2502,7 +2502,7 @@ class docBuilder: output.write(" </letter>\n") output.write(" </chunk>\n") if count != 0: - chunks.append(["chunk%s" % (chunk -1), first_letter, letter]) + chunks.append(["chunk%s" % (chunk - 1), first_letter, letter]) output.write(" <chunks>\n") for ch in chunks: output.write(" <chunk name='%s' start='%s' end='%s'/>\n" % ( diff --git a/docs/index.py b/docs/index.py index e2f9185c3..5224324b1 100755 --- a/docs/index.py +++ b/docs/index.py @@ -58,7 +58,7 @@ libxml2.registerErrorHandler(callback, None) # The dictionary of tables required and the SQL command needed # to create them # -TABLES={ +TABLES = { "symbols": """CREATE TABLE symbols ( name varchar(255) BINARY NOT NULL, module varchar(255) BINARY NOT NULL, @@ -116,8 +116,8 @@ TABLES={ # # The XML API description file to parse # -API="libvirt-api.xml" -DB=None +API = "libvirt-api.xml" +DB = None ######################################################################### # # diff --git a/tests/cputestdata/cpu-cpuid.py b/tests/cputestdata/cpu-cpuid.py index b74c3ce93..43c92731a 100755 --- a/tests/cputestdata/cpu-cpuid.py +++ b/tests/cputestdata/cpu-cpuid.py @@ -325,7 +325,7 @@ def formatCpuid(cpuid, path, comment): line = (" <cpuid eax_in='0x%08x' ecx_in='0x%02x' " "eax='0x%08x' ebx='0x%08x' " "ecx='0x%08x' edx='0x%08x'/>\n") - f.write(line %( + f.write(line % ( in_eax, in_ecx, leaf["eax"], leaf["ebx"], leaf["ecx"], leaf["edx"])) f.write("</cpudata>\n") -- 2.14.3

The generate_helper_source() function returns a formatted string. This could be achieved without the use of a local variable "source" and string concatenation. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- src/esx/esx_vi_generator.py | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 17ac43cb7..8f581c41e 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1092,26 +1092,19 @@ class ManagedObject(GenericObject): def generate_helper_source(self): - source = "" - # lookup - source += "/* esxVI_Lookup%s */\n" % self.name - source += "ESX_VI__TEMPLATE__LOOKUP(%s,\n" % self.name - source += "{\n" - - source += self.generate_lookup_code1() - - source += "},\n" - source += "{\n" - - source += self.generate_lookup_code2() - - source += "})\n\n" - - source += "\n\n" - - return source - + return ( + "/* esxVI_Lookup%(name)s */\n" + "ESX_VI__TEMPLATE__LOOKUP(%(name)s,\n" + "{\n" + "%(lookup_code1)s},\n" + "{\n" + "%(lookup_code2)s})" + "\n\n\n\n" + % {"name": self.name, + "lookup_code1": self.generate_lookup_code1(), + "lookup_code2": self.generate_lookup_code2()} + ) class Enum(Type): -- 2.14.3

The function generate_helper_header() only returns a formatted string. This could be achieved without performing string concatenation. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- src/esx/esx_vi_generator.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 8f581c41e..95521fa1e 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -1021,20 +1021,16 @@ class ManagedObject(GenericObject): def generate_helper_header(self): - header = "" - # functions - header += ("int esxVI_Lookup%s(esxVI_Context *ctx, " - "const char *name, " - "esxVI_ManagedObjectReference *root, " - "esxVI_String *selectedPropertyNameList, " - "esxVI_%s **item, " - "esxVI_Occurrence occurrence);\n") \ - % (self.name, self.name) - - header += "\n" - - return header + return ( + "int esxVI_Lookup%(name)s(esxVI_Context *ctx," + " const char *name," + " esxVI_ManagedObjectReference *root," + " esxVI_String *selectedPropertyNameList," + " esxVI_%(name)s **item," + " esxVI_Occurrence occurrence);\n\n" + % {"name": self.name} + ) def generate_source(self): -- 2.14.3

Reduce the number of if-statements and use a single return. Utilise a dictionary to map between occurrences and values. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- src/esx/esx_vi_generator.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 95521fa1e..df913d891 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -75,16 +75,17 @@ class Member: def get_occurrence_comment(self): - if self.occurrence == OCCURRENCE__REQUIRED_ITEM: - return "/* required */" - elif self.occurrence == OCCURRENCE__REQUIRED_LIST: - return "/* required, list */" - elif self.occurrence == OCCURRENCE__OPTIONAL_ITEM: - return "/* optional */" - elif self.occurrence == OCCURRENCE__OPTIONAL_LIST: - return "/* optional, list */" + occurrence_map = { + OCCURRENCE__REQUIRED_ITEM: "/* required */", + OCCURRENCE__REQUIRED_LIST: "/* required, list */", + OCCURRENCE__OPTIONAL_ITEM: "/* optional */", + OCCURRENCE__OPTIONAL_LIST: "/* optional, list */" + } + try: + return occurrence_map[self.occurrence] + except KeyError: + raise ValueError("unknown occurrence value '%s'" % self.occurrence) - raise ValueError("unknown occurrence value '%s'" % self.occurrence) -- 2.14.3

Generate whitespace using the standard function ljust() that is available in both Py3 [1] and Py2 [2]. 1: https://docs.python.org/3/library/stdtypes.html?highlight=strip#str.ljust 2: https://docs.python.org/2.7/library/string.html#string.ljust Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- src/esx/esx_vi_generator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index df913d891..31bd10fb1 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -49,10 +49,7 @@ separator = "/* " + ("* " * 37) + "*\n" def aligned(left, right, length=59): - while len(left) < length: - left += " " - - return left + right + return left.ljust(length, ' ') + right -- 2.14.3

The functions like isalnum(), isalpha(), isdigit(), etc. are also available in Python, however `make syntax-check` do not intend to prohibit them. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- cfg.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/cfg.mk b/cfg.mk index bc8088d7c..4aa26d434 100644 --- a/cfg.mk +++ b/cfg.mk @@ -496,6 +496,7 @@ ctype_re = isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|islower\ sc_avoid_ctype_macros: @prohibit='\b($(ctype_re)) *\(' \ + in_vc_files='\.[ch]$$' \ halt='use c-ctype.h instead of ctype macros' \ $(_sc_search_regexp) -- 2.14.3

Improve readability by reducing the complexity and length of conditional statements. Example: The following condition: if (o >= 97 and o <= 122) or (o >= 65 and o <= 90) or (o >= 48 and o <= 57) or (" \t(){}:;,+-*/%&!|[]=><".find(line[i]) == -1): Will be True for every character that is not in string: " \t(){}:;,+-*/%&!|[]=><" Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index a11c2d145..51df9dc30 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -386,14 +386,13 @@ class index: def merge_public(self, idx): for id in idx.functions.keys(): if id in self.functions: + up = idx.functions[id] # check that function condition agrees with header - if idx.functions[id].conditionals != \ - self.functions[id].conditionals: + if up.conditionals != self.functions[id].conditionals: self.warning("Header condition differs from Function for %s:" \ % id) self.warning(" H: %s" % self.functions[id].conditionals) - self.warning(" C: %s" % idx.functions[id].conditionals) - up = idx.functions[id] + self.warning(" C: %s" % up.conditionals) self.functions[id].update(None, up.module, up.type, up.info, up.extra) # else: # print("Function %s from %s is not declared in headers" % ( @@ -515,7 +514,7 @@ class CLexer: self.last = ('string', tok) return self.last - if l >= 2 and line[0] == '/' and line[1] == '*': + if line.startswith("/*"): line = line[2:] found = 0 tok = "" @@ -539,7 +538,7 @@ class CLexer: return None self.last = ('comment', tok) return self.last - if l >= 2 and line[0] == '/' and line[1] == '/': + if line.startswith("//"): line = line[2:] self.last = ('comment', line) return self.last @@ -564,28 +563,23 @@ class CLexer: if line[i] == ' ' or line[i] == '\t': i = i + 1 continue - o = ord(line[i]) - if (o >= 97 and o <= 122) or (o >= 65 and o <= 90) or \ - (o >= 48 and o <= 57): + if line[i].isalnum(): s = i while i < l: - o = ord(line[i]) - if (o >= 97 and o <= 122) or (o >= 65 and o <= 90) or \ - (o >= 48 and o <= 57) or \ - (" \t(){}:;,+-*/%&!|[]=><".find(line[i]) == -1): + if line[i] not in " \t(){}:;,+-*/%&!|[]=><": i = i + 1 else: break self.tokens.append(('name', line[s:i])) continue - if "(){}:;,[]".find(line[i]) != -1: + if line[i] in "(){}:;,[]": # if line[i] == '(' or line[i] == ')' or line[i] == '{' or \ # line[i] == '}' or line[i] == ':' or line[i] == ';' or \ # line[i] == ',' or line[i] == '[' or line[i] == ']': self.tokens.append(('sep', line[i])) i = i + 1 continue - if "+-*><=/%&!|.".find(line[i]) != -1: + if line[i] in "+-*><=/%&!|.": # if line[i] == '+' or line[i] == '-' or line[i] == '*' or \ # line[i] == '>' or line[i] == '<' or line[i] == '=' or \ # line[i] == '/' or line[i] == '%' or line[i] == '&' or \ @@ -597,8 +591,7 @@ class CLexer: continue j = i + 1 - if j < l and ( - "+-*><=/%&!|".find(line[j]) != -1): + if j < l and line[j] in "+-*><=/%&!|": # line[j] == '+' or line[j] == '-' or line[j] == '*' or \ # line[j] == '>' or line[j] == '<' or line[j] == '=' or \ # line[j] == '/' or line[j] == '%' or line[j] == '&' or \ @@ -611,10 +604,7 @@ class CLexer: continue s = i while i < l: - o = ord(line[i]) - if (o >= 97 and o <= 122) or (o >= 65 and o <= 90) or \ - (o >= 48 and o <= 57) or \ - (" \t(){}:;,+-*/%&!|[]=><".find(line[i]) == -1): + if line[i] not in " \t(){}:;,+-*/%&!|[]=><": # line[i] != ' ' and line[i] != '\t' and # line[i] != '(' and line[i] != ')' and # line[i] != '{' and line[i] != '}' and @@ -1555,10 +1545,8 @@ class CParser: if token is None: return token - while token[0] == "name" and ( - token[1] == "const" or \ - token[1] == "unsigned" or \ - token[1] == "signed"): + while (token[0] == "name" and + token[1] in ["const", "unsigned", "signed"]): if self.type == "": self.type = token[1] else: @@ -2402,8 +2390,7 @@ class docBuilder: pass typ = sorted(funcs.keys()) for type in typ: - if type == '' or type == 'void' or type == "int" or \ - type == "char *" or type == "const char *": + if type in ['', "void", "int", "char *", "const char *"]: continue output.write(" <type name='%s'>\n" % (type)) ids = funcs[type] @@ -2431,8 +2418,7 @@ class docBuilder: pass typ = sorted(funcs.keys()) for type in typ: - if type == '' or type == 'void' or type == "int" or \ - type == "char *" or type == "const char *": + if type in ['', "void", "int", "char *", "const char *"]: continue output.write(" <type name='%s'>\n" % (type)) ids = sorted(funcs[type]) -- 2.14.3

PEP8 recommends not having spaces around = in a keyword argument or a default parameter value. https://www.python.org/dev/peps/pep-0008/#other-recommendations Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 24 ++++++++++++------------ docs/index.py | 26 +++++++++++++------------- src/esx/esx_vi_generator.py | 4 ++-- tests/cputestdata/cpu-cpuid.py | 4 ++-- tests/cputestdata/cpu-reformat.py | 2 +- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 51df9dc30..50ddf372d 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -137,8 +137,8 @@ def uniq(items): return k class identifier: - def __init__(self, name, header=None, module=None, type=None, lineno = 0, - info=None, extra=None, conditionals = None): + def __init__(self, name, header=None, module=None, type=None, lineno=0, + info=None, extra=None, conditionals=None): self.name = name self.header = header self.module = module @@ -209,7 +209,7 @@ class identifier: def get_conditionals(self): return self.conditionals - def update(self, header, module, type = None, info = None, extra=None, + def update(self, header, module, type=None, info=None, extra=None, conditionals=None): if self.name == debugsym and not quiet: print("=> update %s : %s" % (debugsym, (module, type, info, @@ -228,7 +228,7 @@ class identifier: self.set_conditionals(conditionals) class index: - def __init__(self, name = "noname"): + def __init__(self, name="noname"): self.name = name self.identifiers = {} self.functions = {} @@ -247,7 +247,7 @@ class index: warnings = warnings + 1 print(msg) - def add_ref(self, name, header, module, static, type, lineno, info=None, extra=None, conditionals = None): + def add_ref(self, name, header, module, static, type, lineno, info=None, extra=None, conditionals=None): if name[0:2] == '__': return None d = None @@ -269,7 +269,7 @@ class index: return d - def add(self, name, header, module, static, type, lineno, info=None, extra=None, conditionals = None): + def add(self, name, header, module, static, type, lineno, info=None, extra=None, conditionals=None): if name[0:2] == '__': return None d = None @@ -629,7 +629,7 @@ class CLexer: class CParser: """The C module parser""" - def __init__(self, filename, idx = None): + def __init__(self, filename, idx=None): self.filename = filename if len(filename) > 2 and filename[-2:] == '.h': self.is_header = 1 @@ -661,7 +661,7 @@ class CParser: def lineno(self): return self.lexer.getlineno() - def index_add(self, name, module, static, type, info=None, extra = None): + def index_add(self, name, module, static, type, info=None, extra=None): if self.is_header == 1: self.index.add(name, module, module, static, type, self.lineno(), info, extra, self.conditionals) @@ -670,7 +670,7 @@ class CParser: info, extra, self.conditionals) def index_add_ref(self, name, module, static, type, info=None, - extra = None): + extra=None): if self.is_header == 1: self.index.add_ref(name, module, module, static, type, self.lineno(), info, extra, self.conditionals) @@ -763,7 +763,7 @@ class CParser: # # Parse a comment block associate to a typedef # - def parseTypeComment(self, name, quiet = 0): + def parseTypeComment(self, name, quiet=0): if name[0:2] == '__': quiet = 1 @@ -808,7 +808,7 @@ class CParser: # # Parse a comment block associate to a macro # - def parseMacroComment(self, name, quiet = 0): + def parseMacroComment(self, name, quiet=0): global ignored_macros if name[0:2] == '__': @@ -885,7 +885,7 @@ class CParser: # parameters descriptions, finally returns a block as complete # as possible # - def mergeFunctionComment(self, name, description, quiet = 0): + def mergeFunctionComment(self, name, description, quiet=0): global ignored_functions if name == 'main': diff --git a/docs/index.py b/docs/index.py index 5224324b1..0d07ca4d0 100755 --- a/docs/index.py +++ b/docs/index.py @@ -144,7 +144,7 @@ def createTable(db, name): return -1 return ret -def checkTables(db, verbose = 1): +def checkTables(db, verbose=1): global TABLES if db is None: @@ -188,7 +188,7 @@ def checkTables(db, verbose = 1): pass return 0 -def openMySQL(db="libvir", passwd=None, verbose = 1): +def openMySQL(db="libvir", passwd=None, verbose=1): global DB if passwd is None: @@ -275,25 +275,25 @@ def updateSymbol(name, module, type, desc): return ret -def addFunction(name, module, desc = ""): +def addFunction(name, module, desc=""): return updateSymbol(name, module, 'function', desc) -def addMacro(name, module, desc = ""): +def addMacro(name, module, desc=""): return updateSymbol(name, module, 'macro', desc) -def addEnum(name, module, desc = ""): +def addEnum(name, module, desc=""): return updateSymbol(name, module, 'enum', desc) -def addStruct(name, module, desc = ""): +def addStruct(name, module, desc=""): return updateSymbol(name, module, 'struct', desc) -def addConst(name, module, desc = ""): +def addConst(name, module, desc=""): return updateSymbol(name, module, 'const', desc) -def addType(name, module, desc = ""): +def addType(name, module, desc=""): return updateSymbol(name, module, 'type', desc) -def addFunctype(name, module, desc = ""): +def addFunctype(name, module, desc=""): return updateSymbol(name, module, 'functype', desc) def addPage(resource, title): @@ -1055,7 +1055,7 @@ def analyzeHTMLPages(): import time -def getXMLDateArchive(t = None): +def getXMLDateArchive(t=None): if t is None: t = time.time() T = time.gmtime(t) @@ -1064,7 +1064,7 @@ def getXMLDateArchive(t = None): url = "http://www.redhat.com/archives/libvir-list/%d-%s/date.html" % (year, month) return url -def scanXMLMsgArchive(url, title, force = 0): +def scanXMLMsgArchive(url, title, force=0): if url is None or title is None: return 0 @@ -1094,7 +1094,7 @@ def scanXMLMsgArchive(url, title, force = 0): return 1 -def scanXMLDateArchive(t = None, force = 0): +def scanXMLDateArchive(t=None, force=0): global wordsDictArchive wordsDictArchive = {} @@ -1138,7 +1138,7 @@ def scanXMLDateArchive(t = None, force = 0): # Main code: open the DB, the API XML and analyze it # # # ######################################################################### -def analyzeArchives(t = None, force = 0): +def analyzeArchives(t=None, force=0): global wordsDictArchive ret = scanXMLDateArchive(t, force) diff --git a/src/esx/esx_vi_generator.py b/src/esx/esx_vi_generator.py index 31bd10fb1..28d440a6d 100755 --- a/src/esx/esx_vi_generator.py +++ b/src/esx/esx_vi_generator.py @@ -119,7 +119,7 @@ class Parameter(Member): return aligned(string, self.get_occurrence_comment() + "\n") - def generate_return(self, offset = 0, end_of_line = ";"): + def generate_return(self, offset=0, end_of_line=";"): if self.occurrence == OCCURRENCE__IGNORED: raise ValueError("invalid function parameter occurrence value '%s'" % self.occurrence) @@ -610,7 +610,7 @@ class Object(GenericObject): return source - def generate_deep_copy_code(self, add_banner = False): + def generate_deep_copy_code(self, add_banner=False): source = "" if self.extends is not None: diff --git a/tests/cputestdata/cpu-cpuid.py b/tests/cputestdata/cpu-cpuid.py index 43c92731a..c45118512 100755 --- a/tests/cputestdata/cpu-cpuid.py +++ b/tests/cputestdata/cpu-cpuid.py @@ -343,12 +343,12 @@ def convert(path): with open(path, "w") as f: json.dump({"return": {"model": {"name": "base", "props": props}}, "id": "model-expansion"}, - f, indent = 2, separators = (',', ': ')) + f, indent=2, separators=(',', ': ')) f.write("\n") for chunk in rest: f.write("\n") - json.dump(chunk, f, indent = 2, separators = (',', ': ')) + json.dump(chunk, f, indent=2, separators=(',', ': ')) f.write("\n") diff --git a/tests/cputestdata/cpu-reformat.py b/tests/cputestdata/cpu-reformat.py index 999ef1698..33976e775 100755 --- a/tests/cputestdata/cpu-reformat.py +++ b/tests/cputestdata/cpu-reformat.py @@ -5,5 +5,5 @@ import json dec = json.JSONDecoder() data, pos = dec.raw_decode(sys.stdin.read()) -json.dump(data, sys.stdout, indent = 2, separators = (',', ': ')) +json.dump(data, sys.stdout, indent=2, separators=(',', ': ')) print -- 2.14.3

Lists in Python are mutable and when used as a default value of a parameter for class constructor, its value will be shared between all class instances. Example: class Test: def __init__(self, mylist=[]): self.mylist = mylist A = Test() B = Test() A.mylist.append("mylist from instance A") print(B.mylist) # Will print ['mylist from instance A'] Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- src/hyperv/hyperv_wmi_generator.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hyperv/hyperv_wmi_generator.py b/src/hyperv/hyperv_wmi_generator.py index 6e98f0a89..0dcd9e438 100755 --- a/src/hyperv/hyperv_wmi_generator.py +++ b/src/hyperv/hyperv_wmi_generator.py @@ -44,9 +44,9 @@ class WmiClass: to. """ - def __init__(self, name, versions = []): + def __init__(self, name, versions=None): self.name = name - self.versions = versions + self.versions = versions if versions else list() self.common = None -- 2.14.3

Use a set (unordered collections of unique elements) [1] to remove repeated elements in a list. 1: https://docs.python.org/3/tutorial/datastructures.html#sets Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 50ddf372d..1619c8836 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -130,11 +130,7 @@ def escape(raw): return raw def uniq(items): - d = {} - for item in items: - d[item] = 1 - k = sorted(d.keys()) - return k + return sorted(set(items)) class identifier: def __init__(self, name, header=None, module=None, type=None, lineno=0, -- 2.14.3

The uniq() function returns a sorted list, there is no need to sort this list again. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 1619c8836..e5e4bc83d 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -2318,8 +2318,7 @@ class docBuilder: if desc.find("DEPRECATED") != -1: output.write(" <deprecated/>\n") - ids = sorted(dict.macros.keys()) - for id in uniq(ids): + for id in uniq(dict.macros.keys()): # Macros are sometime used to masquerade other types. if id in dict.functions: continue @@ -2334,20 +2333,15 @@ class docBuilder: if id in dict.enums: continue output.write(" <exports symbol='%s' type='macro'/>\n" % (id)) - ids = sorted(dict.enums.keys()) - for id in uniq(ids): + for id in uniq(dict.enums.keys()): output.write(" <exports symbol='%s' type='enum'/>\n" % (id)) - ids = sorted(dict.typedefs.keys()) - for id in uniq(ids): + for id in uniq(dict.typedefs.keys()): output.write(" <exports symbol='%s' type='typedef'/>\n" % (id)) - ids = sorted(dict.structs.keys()) - for id in uniq(ids): + for id in uniq(dict.structs.keys()): output.write(" <exports symbol='%s' type='struct'/>\n" % (id)) - ids = sorted(dict.variables.keys()) - for id in uniq(ids): + for id in uniq(dict.variables.keys()): output.write(" <exports symbol='%s' type='variable'/>\n" % (id)) - ids = sorted(dict.functions.keys()) - for id in uniq(ids): + for id in uniq(dict.functions.keys()): output.write(" <exports symbol='%s' type='function'/>\n" % (id)) output.write(" </file>\n") @@ -2363,7 +2357,6 @@ class docBuilder: list(dict.typedefs.keys()) + \ list(dict.structs.keys()) + \ list(dict.enums.keys())) - ids.sort() for id in ids: output.write(" <ref name='%s'/>\n" % (id)) output.write(" </file>\n") -- 2.14.3

Replace the print statement, that is only available in Py2, with a print function that is available in both Py2 and Py3 and drop the explicit python version in the shebang. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- tests/cputestdata/cpu-reformat.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cputestdata/cpu-reformat.py b/tests/cputestdata/cpu-reformat.py index 33976e775..d4ed8d811 100755 --- a/tests/cputestdata/cpu-reformat.py +++ b/tests/cputestdata/cpu-reformat.py @@ -1,4 +1,4 @@ -#!/usr/bin/env python2 +#!/usr/bin/env python import sys import json @@ -6,4 +6,4 @@ import json dec = json.JSONDecoder() data, pos = dec.raw_decode(sys.stdin.read()) json.dump(data, sys.stdout, indent=2, separators=(',', ': ')) -print +print("\n") -- 2.14.3

Backslash between brackets in Python is redundant. [1] 1: https://lintlyci.github.io/Flake8Rules/rules/E502.html Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index e5e4bc83d..acfbcafb2 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -385,8 +385,8 @@ class index: up = idx.functions[id] # check that function condition agrees with header if up.conditionals != self.functions[id].conditionals: - self.warning("Header condition differs from Function for %s:" \ - % id) + self.warning("Header condition differs from Function" + " for %s:" % id) self.warning(" H: %s" % self.functions[id].conditionals) self.warning(" C: %s" % up.conditionals) self.functions[id].update(None, up.module, up.type, up.info, up.extra) @@ -1882,7 +1882,7 @@ class CParser: raise Exception() except: self.error(("struct '%s' is not allowed to contain long " - "field '%s', use long long instead") \ + "field '%s', use long long instead") % (name, field[1])) # @@ -1939,8 +1939,8 @@ class CParser: if token[1] == "[": type = type + token[1] token = self.token() - while token is not None and (token[0] != "sep" or \ - token[1] != ";"): + while token is not None and (token[0] != "sep" or + token[1] != ";"): type = type + token[1] token = self.token() @@ -1954,9 +1954,9 @@ class CParser: token = self.parseBlock(token) else: self.comment = None - while token is not None and (token[0] != "sep" or \ - (token[1] != ';' and token[1] != ',')): - token = self.token() + while token is not None and (token[0] != "sep" or + token[1] not in ',;'): + token = self.token() self.comment = None if token is None or token[0] != "sep" or (token[1] != ';' and token[1] != ','): @@ -2351,11 +2351,11 @@ class docBuilder: module = self.modulename_file(file) output.write(" <file name='%s'>\n" % (module)) dict = self.headers[file] - ids = uniq(list(dict.functions.keys()) + \ - list(dict.variables.keys()) + \ - list(dict.macros.keys()) + \ - list(dict.typedefs.keys()) + \ - list(dict.structs.keys()) + \ + ids = uniq(list(dict.functions.keys()) + + list(dict.variables.keys()) + + list(dict.macros.keys()) + + list(dict.typedefs.keys()) + + list(dict.structs.keys()) + list(dict.enums.keys())) for id in ids: output.write(" <ref name='%s'/>\n" % (id)) -- 2.14.3

PEP8 recommends that the number of spaces used for indentation of Python code to be a multiple of four [1] [2]. 1: https://lintlyci.github.io/Flake8Rules/rules/E111.html 2: https://lintlyci.github.io/Flake8Rules/rules/E114.html Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 296 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 150 insertions(+), 146 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index acfbcafb2..90d944ccf 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -248,11 +248,12 @@ class index: return None d = None try: - d = self.identifiers[name] - d.update(header, module, type, lineno, info, extra, conditionals) + d = self.identifiers[name] + d.update(header, module, type, lineno, info, extra, conditionals) except: - d = identifier(name, header, module, type, lineno, info, extra, conditionals) - self.identifiers[name] = d + d = identifier(name, header, module, type, lineno, info, extra, + conditionals) + self.identifiers[name] = d if d is not None and static == 1: d.set_static(1) @@ -265,16 +266,18 @@ class index: return d - def add(self, name, header, module, static, type, lineno, info=None, extra=None, conditionals=None): + def add(self, name, header, module, static, type, lineno, info=None, + extra=None, conditionals=None): if name[0:2] == '__': return None d = None try: - d = self.identifiers[name] - d.update(header, module, type, lineno, info, extra, conditionals) + d = self.identifiers[name] + d.update(header, module, type, lineno, info, extra, conditionals) except: - d = identifier(name, header, module, type, lineno, info, extra, conditionals) - self.identifiers[name] = d + d = identifier(name, header, module, type, lineno, info, extra, + conditionals) + self.identifiers[name] = d if d is not None and static == 1: d.set_static(1) @@ -308,92 +311,93 @@ class index: def merge(self, idx): for id in idx.functions.keys(): - # - # macro might be used to override functions or variables - # definitions - # - if id in self.macros: - del self.macros[id] - if id in self.functions: - self.warning("function %s from %s redeclared in %s" % ( + # + # macro might be used to override functions or variables + # definitions + # + if id in self.macros: + del self.macros[id] + if id in self.functions: + self.warning("function %s from %s redeclared in %s" % ( id, self.functions[id].header, idx.functions[id].header)) - else: - self.functions[id] = idx.functions[id] - self.identifiers[id] = idx.functions[id] + else: + self.functions[id] = idx.functions[id] + self.identifiers[id] = idx.functions[id] for id in idx.variables.keys(): - # - # macro might be used to override functions or variables - # definitions - # - if id in self.macros: - del self.macros[id] - if id in self.variables: - self.warning("variable %s from %s redeclared in %s" % ( + # + # macro might be used to override functions or variables + # definitions + # + if id in self.macros: + del self.macros[id] + if id in self.variables: + self.warning("variable %s from %s redeclared in %s" % ( id, self.variables[id].header, idx.variables[id].header)) - else: - self.variables[id] = idx.variables[id] - self.identifiers[id] = idx.variables[id] + else: + self.variables[id] = idx.variables[id] + self.identifiers[id] = idx.variables[id] for id in idx.structs.keys(): - if id in self.structs: - self.warning("struct %s from %s redeclared in %s" % ( + if id in self.structs: + self.warning("struct %s from %s redeclared in %s" % ( id, self.structs[id].header, idx.structs[id].header)) - else: - self.structs[id] = idx.structs[id] - self.identifiers[id] = idx.structs[id] + else: + self.structs[id] = idx.structs[id] + self.identifiers[id] = idx.structs[id] for id in idx.unions.keys(): - if id in self.unions: - print("union %s from %s redeclared in %s" % ( + if id in self.unions: + print("union %s from %s redeclared in %s" % ( id, self.unions[id].header, idx.unions[id].header)) - else: - self.unions[id] = idx.unions[id] - self.identifiers[id] = idx.unions[id] + else: + self.unions[id] = idx.unions[id] + self.identifiers[id] = idx.unions[id] for id in idx.typedefs.keys(): - if id in self.typedefs: - self.warning("typedef %s from %s redeclared in %s" % ( + if id in self.typedefs: + self.warning("typedef %s from %s redeclared in %s" % ( id, self.typedefs[id].header, idx.typedefs[id].header)) - else: - self.typedefs[id] = idx.typedefs[id] - self.identifiers[id] = idx.typedefs[id] + else: + self.typedefs[id] = idx.typedefs[id] + self.identifiers[id] = idx.typedefs[id] for id in idx.macros.keys(): - # - # macro might be used to override functions or variables - # definitions - # - if id in self.variables: - continue - if id in self.functions: - continue - if id in self.enums: - continue - if id in self.macros: - self.warning("macro %s from %s redeclared in %s" % ( + # + # macro might be used to override functions or variables + # definitions + # + if id in self.variables: + continue + if id in self.functions: + continue + if id in self.enums: + continue + if id in self.macros: + self.warning("macro %s from %s redeclared in %s" % ( id, self.macros[id].header, idx.macros[id].header)) - else: - self.macros[id] = idx.macros[id] - self.identifiers[id] = idx.macros[id] + else: + self.macros[id] = idx.macros[id] + self.identifiers[id] = idx.macros[id] for id in idx.enums.keys(): - if id in self.enums: - self.warning("enum %s from %s redeclared in %s" % ( + if id in self.enums: + self.warning("enum %s from %s redeclared in %s" % ( id, self.enums[id].header, idx.enums[id].header)) - else: - self.enums[id] = idx.enums[id] - self.identifiers[id] = idx.enums[id] + else: + self.enums[id] = idx.enums[id] + self.identifiers[id] = idx.enums[id] def merge_public(self, idx): for id in idx.functions.keys(): - if id in self.functions: - up = idx.functions[id] - # check that function condition agrees with header - if up.conditionals != self.functions[id].conditionals: - self.warning("Header condition differs from Function" - " for %s:" % id) - self.warning(" H: %s" % self.functions[id].conditionals) - self.warning(" C: %s" % up.conditionals) - self.functions[id].update(None, up.module, up.type, up.info, up.extra) - # else: - # print("Function %s from %s is not declared in headers" % ( - # id, idx.functions[id].module)) - # TODO: do the same for variables. + if id in self.functions: + up = idx.functions[id] + # check that function condition agrees with header + if up.conditionals != self.functions[id].conditionals: + self.warning("Header condition differs from Function" + " for %s:" % id) + self.warning(" H: %s" % self.functions[id].conditionals) + self.warning(" C: %s" % up.conditionals) + self.functions[id].update(None, up.module, up.type, up.info, + up.extra) + # else: + # print("Function %s from %s is not declared in headers" % ( + # id, idx.functions[id].module)) + # TODO: do the same for variables. def analyze_dict(self, type, dict): count = 0 @@ -725,7 +729,7 @@ class CParser: elif line[i] == '*': return line[:i] + line[i + 1:] else: - return line + return line return line def cleanupComment(self): @@ -876,11 +880,11 @@ class CParser: return((args, desc)) - # - # Parse a comment block and merge the information found in the - # parameters descriptions, finally returns a block as complete - # as possible - # + # + # Parse a comment block and merge the information found in the + # parameters descriptions, finally returns a block as complete + # as possible + # def mergeFunctionComment(self, name, description, quiet=0): global ignored_functions @@ -988,9 +992,9 @@ class CParser: desc = desc.strip() if quiet == 0: - # - # report missing comments - # + # + # report missing comments + # i = 0 while i < nbargs: if args[i][2] is None and args[i][0] != "void" and args[i][1] is not None: @@ -1022,7 +1026,7 @@ class CParser: if token is None: return None if token[0] == 'preproc': - # TODO macros with arguments + # TODO macros with arguments name = token[1] lst = [] token = self.lexer.token() @@ -1107,11 +1111,11 @@ class CParser: token = self.lexer.token() return token - # - # token acquisition on top of the lexer, it handle internally - # preprocessor and comments since they are logically not part of - # the program structure. - # + # + # token acquisition on top of the lexer, it handle internally + # preprocessor and comments since they are logically not part of + # the program structure. + # def push(self, tok): self.lexer.push(tok) @@ -1148,9 +1152,9 @@ class CParser: return token return None - # - # Parse a typedef, it records the type and its name. - # + # + # Parse a typedef, it records the type and its name. + # def parseTypedef(self, token): if token is None: return None @@ -1160,7 +1164,7 @@ class CParser: return None base_type = self.type type = base_type - #self.debug("end typedef type", token) + # self.debug("end typedef type", token) while token is not None: if token[0] == "name": name = token[1] @@ -1185,7 +1189,7 @@ class CParser: else: self.error("parsing typedef: expecting a name") return token - #self.debug("end typedef", token) + # self.debug("end typedef", token) if token is not None and token[0] == 'sep' and token[1] == ',': type = base_type token = self.token() @@ -1203,10 +1207,10 @@ class CParser: token = self.token() return token - # - # Parse a C code block, used for functions it parse till - # the balancing } included - # + # + # Parse a C code block, used for functions it parse till + # the balancing } included + # def parseBlock(self, token): while token is not None: if token[0] == "sep" and token[1] == "{": @@ -1242,27 +1246,27 @@ class CParser: token = self.token() return token - # - # Parse a C struct definition till the balancing } - # + # + # Parse a C struct definition till the balancing } + # def parseStruct(self, token): fields = [] - #self.debug("start parseStruct", token) + # self.debug("start parseStruct", token) while token is not None: if token[0] == "sep" and token[1] == "{": token = self.token() token = self.parseTypeBlock(token) elif token[0] == "sep" and token[1] == "}": self.struct_fields = fields - #self.debug("end parseStruct", token) - #print(fields) + # self.debug("end parseStruct", token) + # print(fields) token = self.token() return token else: base_type = self.type - #self.debug("before parseType", token) + # self.debug("before parseType", token) token = self.parseType(token) - #self.debug("after parseType", token) + # self.debug("after parseType", token) if token is not None and token[0] == "name": fname = token[1] token = self.token() @@ -1293,13 +1297,13 @@ class CParser: token = self.token() self.type = base_type self.struct_fields = fields - #self.debug("end parseStruct", token) - #print(fields) + # self.debug("end parseStruct", token) + # print(fields) return token - # - # Parse a C union definition till the balancing } - # + # + # Parse a C union definition till the balancing } + # def parseUnion(self, token): fields = [] # self.debug("start parseUnion", token) @@ -1347,9 +1351,9 @@ class CParser: # print(fields) return token - # - # Parse a C enum block, parse till the balancing } - # + # + # Parse a C enum block, parse till the balancing } + # def parseEnumBlock(self, token): self.enums = [] name = None @@ -1512,10 +1516,10 @@ class CParser: return token - # - # Parse a C definition block, used for structs or unions it parse till - # the balancing } - # + # + # Parse a C definition block, used for structs or unions it parse till + # the balancing } + # def parseTypeBlock(self, token): while token is not None: if token[0] == "sep" and token[1] == "{": @@ -1528,11 +1532,11 @@ class CParser: token = self.token() return token - # - # Parse a type: the fact that the type name can either occur after - # the definition or within the definition makes it a little harder - # if inside, the name token is pushed back before returning - # + # + # Parse a type: the fact that the type name can either occur after + # the definition or within the definition makes it a little harder + # if inside, the name token is pushed back before returning + # def parseType(self, token): self.type = "" self.struct_fields = [] @@ -1710,9 +1714,9 @@ class CParser: self.type = self.type + " " + token[1] token = self.token() - # - # if there is a parenthesis here, this means a function type - # + # + # if there is a parenthesis here, this means a function type + # if token is not None and token[0] == "sep" and token[1] == '(': self.type = self.type + token[1] token = self.token() @@ -1743,9 +1747,9 @@ class CParser: token = nametok return token - # - # do some lookahead for arrays - # + # + # do some lookahead for arrays + # if token is not None and token[0] == "name": nametok = token token = self.token() @@ -1765,7 +1769,7 @@ class CParser: self.error("parsing array type, ']' expected", token) return token elif token is not None and token[0] == "sep" and token[1] == ':': - # remove :12 in case it's a limited int size + # remove :12 in case it's a limited int size token = self.token() token = self.token() self.lexer.push(token) @@ -1773,9 +1777,9 @@ class CParser: return token - # - # Parse a signature: '(' has been parsed and we scan the type definition - # up to the ')' included + # + # Parse a signature: '(' has been parsed and we scan the type definition + # up to the ')' included def parseSignature(self, token): signature = [] if token is not None and token[0] == "sep" and token[1] == ')': @@ -1791,7 +1795,7 @@ class CParser: token = self.token() continue elif token is not None and token[0] == "sep" and token[1] == ')': - # only the type was provided + # only the type was provided if self.type == "...": signature.append((self.type, "...", None)) else: @@ -1885,10 +1889,10 @@ class CParser: "field '%s', use long long instead") % (name, field[1])) - # - # Parse a global definition, be it a type, variable or function - # the extern "C" blocks are a bit nasty and require it to recurse. - # + # + # Parse a global definition, be it a type, variable or function + # the extern "C" blocks are a bit nasty and require it to recurse. + # def parseGlobal(self, token): static = 0 if token[1] == 'extern': @@ -1945,9 +1949,9 @@ class CParser: token = self.token() if token is not None and token[0] == "op" and token[1] == "=": - # - # Skip the initialization of the variable - # + # + # Skip the initialization of the variable + # token = self.token() if token[0] == 'sep' and token[1] == '{': token = self.token() -- 2.14.3

The method strip_lead_star() removes a single leading asterisk character from a string by ignoring leading whitespace, otherwise it returns the original string. This could be achieved with a single if-statement followed by replace. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 90d944ccf..d5707d5d1 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -721,15 +721,8 @@ class CParser: self.index.info = res def strip_lead_star(self, line): - l = len(line) - i = 0 - while i < l: - if line[i] == ' ' or line[i] == '\t': - i += 1 - elif line[i] == '*': - return line[:i] + line[i + 1:] - else: - return line + if line.lstrip().startswith('*'): + line = line.replace('*', '', 1) return line def cleanupComment(self): -- 2.14.3

Improve readability and reduce complexity the method parseTypeComment(). Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index d5707d5d1..06d556bae 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -756,48 +756,38 @@ class CParser: # # Parse a comment block associate to a typedef # - def parseTypeComment(self, name, quiet=0): + def parseTypeComment(self, name, quiet=False): if name[0:2] == '__': - quiet = 1 - - args = [] - desc = "" + quiet = True if self.comment is None: if not quiet: self.warning("Missing comment for type %s" % (name)) - return((args, desc)) - if self.comment[0] != '*': + return None + if not self.comment.startswith('*'): if not quiet: self.warning("Missing * in type comment for %s" % (name)) - return((args, desc)) + return None + lines = self.comment.split('\n') - if lines[0] == '*': - del lines[0] + # Remove lines that contain only single asterisk + lines[:] = [line for line in lines if line.strip() != '*'] + if lines[0] != "* %s:" % (name): if not quiet: self.warning("Misformatted type comment for %s" % (name)) self.warning(" Expecting '* %s:' got '%s'" % (name, lines[0])) - return((args, desc)) + return None del lines[0] - while len(lines) > 0 and lines[0] == '*': - del lines[0] - desc = "" - while len(lines) > 0: - l = lines[0] - while len(l) > 0 and l[0] == '*': - l = l[1:] - l = l.strip() - desc = desc + " " + l - del lines[0] - desc = desc.strip() + # Concatenate all remaining lines by striping leading asterisks + desc = " ".join([line.lstrip("*").strip() for line in lines]).strip() - if quiet == 0: - if desc == "": - self.warning("Type comment for %s lack description of the macro" % (name)) + if not (quiet or desc): + self.warning("Type comment for %s lack description of the macro" + % (name)) - return(desc) + return desc # # Parse a comment block associate to a macro # -- 2.14.3

Reduce the number of if-statements used to assign a literals to corresponding class variables. Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 06d556bae..f073b36c7 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -283,24 +283,19 @@ class index: d.set_static(1) if d is not None and name is not None and type is not None: - if type == "function": - self.functions[name] = d - elif type == "functype": - self.functions[name] = d - elif type == "variable": - self.variables[name] = d - elif type == "include": - self.includes[name] = d - elif type == "struct": - self.structs[name] = d - elif type == "union": - self.unions[name] = d - elif type == "enum": - self.enums[name] = d - elif type == "typedef": - self.typedefs[name] = d - elif type == "macro": - self.macros[name] = d + type_map = { + "function": self.functions, + "functype": self.functions, + "variable": self.variables, + "include": self.includes, + "struct": self.structs, + "union": self.unions, + "enum": self.enums, + "typedef": self.typedefs, + "macro": self.macros + } + if type in type_map: + type_map[type][name] = d else: self.warning("Unable to register type ", type) -- 2.14.3

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index f073b36c7..98224c7e7 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -471,8 +471,7 @@ class CLexer: return None if line[0] == '#': - self.tokens = list(map((lambda x: ('preproc', x)), - line.split())) + self.tokens = [('preproc', word) for word in line.split()] # We might have whitespace between the '#' and preproc # macro name, so instead of having a single token element -- 2.14.3

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 98224c7e7..4c8fa5740 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -478,8 +478,8 @@ class CLexer: # of '#define' we might end up with '#' and 'define'. This # merges them back together if self.tokens[0][1] == "#": - self.tokens[0] = ('preproc', self.tokens[0][1] + self.tokens[1][1]) - self.tokens = self.tokens[:1] + self.tokens[2:] + self.tokens[0] = ('preproc', "#" + self.tokens[1][1]) + del self.tokens[1] break l = len(line) if line[0] == '"' or line[0] == "'": -- 2.14.3

Improve readability and reduce the complexity of the code that is searching for string tokens (i.e. characters surrounded by a single or double quote). Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 4c8fa5740..94de13b56 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -483,28 +483,16 @@ class CLexer: break l = len(line) if line[0] == '"' or line[0] == "'": - end = line[0] - line = line[1:] - found = 0 - tok = "" - while found == 0: - i = 0 - l = len(line) - while i < l: - if line[i] == end: - self.line = line[i+1:] - line = line[:i] - l = i - found = 1 - break - if line[i] == '\\': - i = i + 1 - i = i + 1 - tok = tok + line - if found == 0: - line = self.getline() - if line is None: - return None + quote = line[0] + i = 1 + while quote not in line[i:]: + i = len(line) + nextline = self.getline() + if nextline is None: + return None + line += nextline + + tok, self.line = line[1:].split(quote, 1) self.last = ('string', tok) return self.last -- 2.14.3

http://pylint-messages.wikidot.com/messages:c0325 Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 74 ++++++++++++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 94de13b56..1f9c8f12c 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -156,7 +156,7 @@ class identifier: if self.static: r = r + " static" if self.module is not None: - r = r + " from %s" % (self.module) + r = r + " from %s" % self.module if self.info is not None: r = r + " " + repr(self.info) if self.extra is not None: @@ -458,7 +458,7 @@ class CLexer: def debug(self): print("Last token: ", self.last) print("Token queue: ", self.tokens) - print("Line %d end: " % (self.lineno), self.line) + print("Line %d end: " % self.lineno, self.line) def token(self): while self.tokens == []: @@ -744,20 +744,20 @@ class CParser: if self.comment is None: if not quiet: - self.warning("Missing comment for type %s" % (name)) + self.warning("Missing comment for type %s" % name) return None if not self.comment.startswith('*'): if not quiet: - self.warning("Missing * in type comment for %s" % (name)) + self.warning("Missing * in type comment for %s" % name) return None lines = self.comment.split('\n') # Remove lines that contain only single asterisk lines[:] = [line for line in lines if line.strip() != '*'] - if lines[0] != "* %s:" % (name): + if lines[0] != "* %s:" % name: if not quiet: - self.warning("Misformatted type comment for %s" % (name)) + self.warning("Misformatted type comment for %s" % name) self.warning(" Expecting '* %s:' got '%s'" % (name, lines[0])) return None del lines[0] @@ -767,7 +767,7 @@ class CParser: if not (quiet or desc): self.warning("Type comment for %s lack description of the macro" - % (name)) + % name) return desc # @@ -786,33 +786,33 @@ class CParser: if self.comment is None: if not quiet: - self.warning("Missing comment for macro %s" % (name)) - return((args, desc)) + self.warning("Missing comment for macro %s" % name) + return args, desc if self.comment[0] != '*': if not quiet: - self.warning("Missing * in macro comment for %s" % (name)) - return((args, desc)) + self.warning("Missing * in macro comment for %s" % name) + return args, desc lines = self.comment.split('\n') if lines[0] == '*': del lines[0] - if lines[0] != "* %s:" % (name): + if lines[0] != "* %s:" % name: if not quiet: - self.warning("Misformatted macro comment for %s" % (name)) + self.warning("Misformatted macro comment for %s" % name) self.warning(" Expecting '* %s:' got '%s'" % (name, lines[0])) - return((args, desc)) + return args, desc del lines[0] while lines[0] == '*': del lines[0] while len(lines) > 0 and lines[0][0:3] == '* @': l = lines[0][3:] try: - (arg, desc) = l.split(':', 1) + arg, desc = l.split(':', 1) desc = desc.strip() arg = arg.strip() except: if not quiet: - self.warning("Misformatted macro comment for %s" % (name)) - self.warning(" problem with '%s'" % (lines[0])) + self.warning("Misformatted macro comment for %s" % name) + self.warning(" problem with '%s'" % lines[0]) del lines[0] continue del lines[0] @@ -841,9 +841,9 @@ class CParser: if quiet == 0: if desc == "": - self.warning("Macro comment for %s lack description of the macro" % (name)) + self.warning("Macro comment for %s lack description of the macro" % name) - return((args, desc)) + return args, desc # # Parse a comment block and merge the information found in the @@ -860,26 +860,26 @@ class CParser: if name in ignored_functions: quiet = 1 - (ret, args) = description + ret, args = description desc = "" retdesc = "" if self.comment is None: if not quiet: - self.warning("Missing comment for function %s" % (name)) - return(((ret[0], retdesc), args, desc)) + self.warning("Missing comment for function %s" % name) + return (ret[0], retdesc), args, desc if self.comment[0] != '*': if not quiet: - self.warning("Missing * in function comment for %s" % (name)) - return(((ret[0], retdesc), args, desc)) + self.warning("Missing * in function comment for %s" % name) + return (ret[0], retdesc), args, desc lines = self.comment.split('\n') if lines[0] == '*': del lines[0] - if lines[0] != "* %s:" % (name): + if lines[0] != "* %s:" % name: if not quiet: - self.warning("Misformatted function comment for %s" % (name)) + self.warning("Misformatted function comment for %s" % name) self.warning(" Expecting '* %s:' got '%s'" % (name, lines[0])) - return(((ret[0], retdesc), args, desc)) + return (ret[0], retdesc), args, desc del lines[0] while lines[0] == '*': del lines[0] @@ -887,13 +887,13 @@ class CParser: while len(lines) > 0 and lines[0][0:3] == '* @': l = lines[0][3:] try: - (arg, desc) = l.split(':', 1) + arg, desc = l.split(':', 1) desc = desc.strip() arg = arg.strip() except: if not quiet: - self.warning("Misformatted function comment for %s" % (name)) - self.warning(" problem with '%s'" % (lines[0])) + self.warning("Misformatted function comment for %s" % name) + self.warning(" problem with '%s'" % lines[0]) del lines[0] continue del lines[0] @@ -966,12 +966,12 @@ class CParser: self.warning("Function comment for %s lacks description of arg %s" % (name, args[i][1])) i = i + 1 if retdesc == "" and ret[0] != "void": - self.warning("Function comment for %s lacks description of return value" % (name)) + self.warning("Function comment for %s lacks description of return value" % name) if desc == "": - self.warning("Function comment for %s lacks description of the function" % (name)) + self.warning("Function comment for %s lacks description of the function" % name) - return(((ret[0], retdesc), args, desc)) + return (ret[0], retdesc), args, desc def parsePreproc(self, token): if debug: @@ -1362,7 +1362,7 @@ class CParser: try: value = "%d" % (int(value) + 1) except: - self.warning("Failed to compute value of enum %s" % (name)) + self.warning("Failed to compute value of enum %s" % name) value = "" if token[0] == "sep" and token[1] == ",": if commentsBeforeVal: @@ -1822,7 +1822,7 @@ class CParser: raise Exception() except: self.error(("function '%s' is not allowed to return long, " - "use long long instead") % (name)) + "use long long instead") % name) for param in signature: if "long" in param[0] and "long long" not in param[0]: @@ -2156,7 +2156,7 @@ class docBuilder: name, escape(desc))) self.indexString(name, desc) else: - output.write(" <arg name='%s'/>\n" % (name)) + output.write(" <arg name='%s'/>\n" % name) output.write(" </macro>\n") def serialize_union(self, output, field, desc): @@ -2195,7 +2195,7 @@ class docBuilder: else: output.write(" <field name='%s' type='%s' info='%s'/>\n" % (field[1], field[0], desc)) except: - self.warning("Failed to serialize struct %s" % (name)) + self.warning("Failed to serialize struct %s" % name) output.write(" </struct>\n") else: output.write("/>\n") -- 2.14.3

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com> --- docs/apibuild.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/docs/apibuild.py b/docs/apibuild.py index 1f9c8f12c..5e218a9ad 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -432,21 +432,17 @@ class CLexer: line = self.input.readline() if not line: return None - self.lineno = self.lineno + 1 - line = line.lstrip() - line = line.rstrip() + self.lineno += 1 + line = line.strip() if line == '': continue while line[-1] == '\\': line = line[:-1] - n = self.input.readline() - self.lineno = self.lineno + 1 - n = n.lstrip() - n = n.rstrip() + n = self.input.readline().strip() + self.lineno += 1 if not n: break - else: - line = line + n + line += n return line def getlineno(self): -- 2.14.3

On Tue, Mar 20, 2018 at 06:48:43AM +0000, Radostin Stoyanov wrote:
Changes since v1 (https://www.redhat.com/archives/libvir-list/2018-March/msg01001.html)
01 - Fixed typo. 03 - Added whitespace after comma in: value = value + re.sub("^(\d+)U$","\\1", token[1]) 05 and 06 - Use printf style format and break strings everywhere there is a newline. 07 - Use try-except instead of an explicit check. 10 - Use isalnum() instead of a regex match, and a few more simplifications were added. 20 - Use try-except instead of an explicit check. 23 - Don't slice `line` in the loop condition.
New patches: 9, 24, 25
The patches have been tested with `make check`, `make syntax-check`. The output of apibuild.py was tested with `cd docs/` followed by:
srcdir=. builddir=. /usr/bin/python2 ./apibuild.py; sha1sum *.xml | sha1sum
and
srcdir=. builddir=. /usr/bin/python3 ./apibuild.py; sha1sum *.xml | sha1sum
before and after the patches were applied.
Radostin Stoyanov (25): apibuild: Use isinstance for type checking apibuild: Split imports on separate lines apibuild: Remove whitespace before ',' and ':' python: Add whitespace around = and % operators esx_vi_generator: Simplify generate_helper_source esx_vi_generator: Simplify generate_helper_header esx_vi_generator: Simplify get_occurrence_comment esx_vi_generator: Simplify alignment function cfg.mk: check ctype_macros only on *.[c|h] files apibuild: Simplify conditional statements python: Remove space around = in keyword args WmiClass: Don't share "versions" between instances apibuild: Simplify uniq function apibuild: Avoid double sorting of ids python3: cpu-reformat: Use the print() function apibuild: Drop backslash between brackets apibuild: Fix indentation not multiple of 4 apibuild: Simplify strip_lead_star() apibuild: Simplify parseTypeComment() apibuild: Simplify type checking of literals apibuild: Use list comprehension insteand of map apibuild: Simplify merging of preproc tokens apibuild: Simplify parsing string tokens apibuild: Remove redundant parentheses apibuild: Simplify getline()
cfg.mk | 1 + docs/apibuild.py | 734 +++++++++++++++++-------------------- docs/index.py | 54 +-- src/esx/esx_vi_generator.py | 183 ++++----- src/hyperv/hyperv_wmi_generator.py | 36 +- tests/cputestdata/cpu-cpuid.py | 6 +- tests/cputestdata/cpu-reformat.py | 6 +- 7 files changed, 472 insertions(+), 548 deletions(-)
I've reviewed all changes and pushed the result. I would still like to see you contribute a patch that extends "make syntax-check" to validate these python style rules to prevent regressions. 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 :|

On Tue, Mar 20, 2018 at 06:48:43AM +0000, Radostin Stoyanov wrote:
Changes since v1 (https://www.redhat.com/archives/libvir-list/2018-March/msg01001.html)
01 - Fixed typo. 03 - Added whitespace after comma in: value = value + re.sub("^(\d+)U$","\\1", token[1]) 05 and 06 - Use printf style format and break strings everywhere there is a newline. 07 - Use try-except instead of an explicit check. 10 - Use isalnum() instead of a regex match, and a few more simplifications were added. 20 - Use try-except instead of an explicit check. 23 - Don't slice `line` in the loop condition.
New patches: 9, 24, 25
The patches have been tested with `make check`, `make syntax-check`. The output of apibuild.py was tested with `cd docs/` followed by:
srcdir=. builddir=. /usr/bin/python2 ./apibuild.py; sha1sum *.xml | sha1sum
and
srcdir=. builddir=. /usr/bin/python3 ./apibuild.py; sha1sum *.xml | sha1sum
before and after the patches were applied.
The apibuild.py script was borrowed from libxml2 IIRC. I don't know what the python3-compatibility status is over there, but it could benefit from some of the cleanups if you're feeling adventurous ;) Jan
participants (3)
-
Daniel P. Berrangé
-
Ján Tomko
-
Radostin Stoyanov