[libvirt] [PATCH 0/4]typo fix and codes improvement in generator.py
This four patches try to fix various typoes, fd leaks and optimize codes in generator.py script. This is the first round. Guannan Ren(4) [PATCH 1/4] python: global variable and debugging improvement for [PATCH 2/4] python: fix typoes and repeated global vars references [PATCH 3/4] python: optimize SAX xml parsing event handler [PATCH 4/4] python: fix fd leak in generator.py python/generator.py | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------------- 1 file changed, 70 insertions(+), 88 deletions(-)
* Put import clause in front of global variables * Sink __name__ == "__main__" to the bottom of this script and support "import generator" * Remove "quiet" and "debug" global variables and use stubs_buiding_debug and xml_parsing_debug variable instead --- python/generator.py | 105 +++++++++++++++++++++++++++------------------------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/python/generator.py b/python/generator.py index e4c9579..246767c 100755 --- a/python/generator.py +++ b/python/generator.py @@ -3,31 +3,20 @@ # generate python wrappers from the XML API description # -functions = {} -lxc_functions = {} -qemu_functions = {} -enums = {} # { enumType: { enumConstant: enumValue } } -lxc_enums = {} # { enumType: { enumConstant: enumValue } } -qemu_enums = {} # { enumType: { enumConstant: enumValue } } - import os import sys import string import re -quiet=True +from xml.sax import ContentHandler +from xml.sax import make_parser -if __name__ == "__main__": - # launched as a script - srcPref = os.path.dirname(sys.argv[0]) - if len(sys.argv) > 1: - python = sys.argv[1] - else: - print "Python binary not specified" - sys.exit(1) -else: - # imported - srcPref = os.path.dirname(__file__) +functions = {} +lxc_functions = {} +qemu_functions = {} +enums = {} # { enumType: { enumConstant: enumValue } } +lxc_enums = {} # { enumType: { enumConstant: enumValue } } +qemu_enums = {} # { enumType: { enumConstant: enumValue } } ####################################################################### # @@ -35,20 +24,17 @@ else: # libvirt API description # ####################################################################### -import os -import xml.sax -debug = 0 - -def getparser(): +def getparser(debug): # Attach parser to an unmarshalling object. return both objects. - target = docParser() - parser = xml.sax.make_parser() + target = docParser(debug) + parser = make_parser() parser.setContentHandler(target) return parser, target -class docParser(xml.sax.handler.ContentHandler): - def __init__(self): +class docParser(ContentHandler): + def __init__(self, debug = False): + self.debug = debug; self._methodname = None self._data = [] self.in_function = 0 @@ -58,24 +44,24 @@ class docParser(xml.sax.handler.ContentHandler): self.characters = self.data def close(self): - if debug: + if self.debug: print "close" def getmethodname(self): return self._methodname def data(self, text): - if debug: + if self.debug: print "data %s" % text self._data.append(text) def cdata(self, text): - if debug: + if self.debug: print "data %s" % text self._data.append(text) def start(self, tag, attrs): - if debug: + if self.debug: print "start %s, %s" % (tag, attrs) if tag == 'function': self._data = [] @@ -132,7 +118,7 @@ class docParser(xml.sax.handler.ContentHandler): qemu_enum(attrs['type'],attrs['name'],attrs['value']) def end(self, tag): - if debug: + if self.debug: print "end %s" % tag if tag == 'function': # fuctions come from source files, hence 'virerror.c' @@ -779,7 +765,7 @@ def print_function_wrapper(module, name, output, export, include): return 0 return 1 -def buildStubs(module): +def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): global py_types global py_return_types global unknown_types @@ -806,14 +792,14 @@ def buildStubs(module): try: f = open(os.path.join(srcPref,api_xml)) data = f.read() - (parser, target) = getparser() + (parser, target) = getparser(xml_parsing_debug) parser.feed(data) parser.close() except IOError, msg: try: f = open(os.path.join(srcPref,"..","docs",api_xml)) data = f.read() - (parser, target) = getparser() + (parser, target) = getparser(xml_parsing_debug) parser.feed(data) parser.close() except IOError, msg: @@ -821,7 +807,7 @@ def buildStubs(module): sys.exit(1) n = len(funcs.keys()) - if not quiet: + if stubs_buiding_debug: print "Found %d functions in %s" % ((n), api_xml) override_api_xml = "%s-override-api.xml" % module @@ -830,13 +816,13 @@ def buildStubs(module): try: f = open(os.path.join(srcPref, override_api_xml)) data = f.read() - (parser, target) = getparser() + (parser, target) = getparser(xml_parsing_debug) parser.feed(data) parser.close() except IOError, msg: print file, ":", msg - if not quiet: + if stubs_buiding_debug: # XXX: This is not right, same function already in @functions # will be overwritten. print "Found %d functions in %s" % ((len(funcs.keys()) - n), override_api_xml) @@ -879,7 +865,7 @@ def buildStubs(module): export.close() wrapper.close() - if not quiet: + if stubs_buiding_debug: print "Generated %d wrapper functions" % nb_wrap if unknown_types: @@ -1955,15 +1941,32 @@ def lxcBuildWrappers(module): fd.close() +if __name__ == "__main__": + # launched as a script -quiet = 0 -if buildStubs("libvirt") < 0: - sys.exit(1) -if buildStubs("libvirt-lxc") < 0: - sys.exit(1) -if buildStubs("libvirt-qemu") < 0: - sys.exit(1) -buildWrappers("libvirt") -lxcBuildWrappers("libvirt-lxc") -qemuBuildWrappers("libvirt-qemu") -sys.exit(0) + stubs_buiding_debug = False + xml_parsing_debug = False + + srcPref = os.path.dirname(sys.argv[0]) + if len(sys.argv) > 1: + python = sys.argv[1] + else: + print "Python binary not specified" + sys.exit(1) + + if buildStubs("libvirt", stubs_buiding_debug, xml_parsing_debug) < 0: + sys.exit(1) + if buildStubs("libvirt-lxc", stubs_buiding_debug, xml_parsing_debug) < 0: + sys.exit(1) + if buildStubs("libvirt-qemu", stubs_buiding_debug, xml_parsing_debug) < 0: + sys.exit(1) + + buildWrappers("libvirt") + lxcBuildWrappers("libvirt-lxc") + qemuBuildWrappers("libvirt-qemu") + sys.exit(0) +else: + # imported + srcPref = os.path.dirname(__file__) + python = sys.executable + assert python -- 1.7.11.2
On 02/28/2013 03:03 AM, Guannan Ren wrote:
* Put import clause in front of global variables * Sink __name__ == "__main__" to the bottom of this script and support "import generator" * Remove "quiet" and "debug" global variables and use stubs_buiding_debug and xml_parsing_debug variable instead
s/buiding/building/
--- python/generator.py | 105 +++++++++++++++++++++++++++------------------------- 1 file changed, 54 insertions(+), 51 deletions(-)
- if not quiet: + if stubs_buiding_debug:
and here, as well
+ + if buildStubs("libvirt", stubs_buiding_debug, xml_parsing_debug) < 0: + sys.exit(1) + if buildStubs("libvirt-lxc", stubs_buiding_debug, xml_parsing_debug) < 0: + sys.exit(1) + if buildStubs("libvirt-qemu", stubs_buiding_debug, xml_parsing_debug) < 0: + sys.exit(1)
And here. My python is too weak to say whether this makes sense, especially not whether it makes sense for 1.0.3 or whether it should wait until after the release. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- python/generator.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/python/generator.py b/python/generator.py index 246767c..39e654b 100755 --- a/python/generator.py +++ b/python/generator.py @@ -781,11 +781,11 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): elif module == "libvirt-lxc": funcs = lxc_functions funcs_failed = lxc_functions_failed - funcs_skipped = functions_skipped + funcs_skipped = lxc_functions_skipped elif module == "libvirt-qemu": funcs = qemu_functions funcs_failed = qemu_functions_failed - funcs_skipped = functions_skipped + funcs_skipped = qemu_functions_skipped api_xml = "%s-api.xml" % module @@ -1205,12 +1205,9 @@ def buildWrappers(module): global function_classes global classes_type global classes_list - global converter_type global primary_classes - global converter_type global classes_ancestor global converter_type - global primary_classes global classes_destructors global functions_noexcept -- 1.7.11.2
On 02/28/2013 03:03 AM, Guannan Ren wrote:
--- python/generator.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
ACK. I don't have to understand python to see that this one makes sense :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
close(), getmethodname(), cdata() are not standard methods from parent class ContentHandler and not being used anywhere in codes, so remove them. In docParser, actually, we are overloading three parent methods startElement(), endElement() and characters(), so I rename back three of these method names for example from start() to startElement() which makes it simpler to read. make other improvements in codes. --- python/generator.py | 53 ++++++++++++++++------------------------------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/python/generator.py b/python/generator.py index 39e654b..7ccd471 100755 --- a/python/generator.py +++ b/python/generator.py @@ -30,41 +30,24 @@ def getparser(debug): target = docParser(debug) parser = make_parser() parser.setContentHandler(target) - return parser, target + return parser class docParser(ContentHandler): def __init__(self, debug = False): self.debug = debug; - self._methodname = None - self._data = [] + self.data = [] self.in_function = 0 - self.startElement = self.start - self.endElement = self.end - self.characters = self.data - - def close(self): - if self.debug: - print "close" - - def getmethodname(self): - return self._methodname - - def data(self, text): - if self.debug: - print "data %s" % text - self._data.append(text) - - def cdata(self, text): + def characters(self, text): if self.debug: print "data %s" % text - self._data.append(text) + self.data.append(text) - def start(self, tag, attrs): + def startElement(self, tag, attrs): if self.debug: print "start %s, %s" % (tag, attrs) if tag == 'function': - self._data = [] + self.data = [] self.in_function = 1 self.function = None self.function_cond = None @@ -80,9 +63,9 @@ class docParser(ContentHandler): if attrs.has_key('module'): self.function_module= attrs['module'] elif tag == 'cond': - self._data = [] + self.data = [] elif tag == 'info': - self._data = [] + self.data = [] elif tag == 'arg': if self.in_function == 1: self.function_arg_name = None @@ -117,7 +100,7 @@ class docParser(ContentHandler): elif attrs['file'] == "libvirt-qemu": qemu_enum(attrs['type'],attrs['name'],attrs['value']) - def end(self, tag): + def endElement(self, tag): if self.debug: print "end %s" % tag if tag == 'function': @@ -167,17 +150,13 @@ class docParser(ContentHandler): self.function_return_info, self.function_return_field] elif tag == 'info': - str = '' - for c in self._data: - str = str + c if self.in_function == 1: - self.function_descr = str + text = ''.join(self.data) + self.function_descr = text elif tag == 'cond': - str = '' - for c in self._data: - str = str + c if self.in_function == 1: - self.function_cond = str + text = ''.join(self.data) + self.function_cond = text def function(name, desc, ret, args, file, module, cond): @@ -792,14 +771,14 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref,api_xml)) data = f.read() - (parser, target) = getparser(xml_parsing_debug) + parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() except IOError, msg: try: f = open(os.path.join(srcPref,"..","docs",api_xml)) data = f.read() - (parser, target) = getparser(xml_parsing_debug) + parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() except IOError, msg: @@ -816,7 +795,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref, override_api_xml)) data = f.read() - (parser, target) = getparser(xml_parsing_debug) + parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() except IOError, msg: -- 1.7.11.2
On 02/28/2013 03:03 AM, Guannan Ren wrote:
close(), getmethodname(), cdata() are not standard methods from parent class ContentHandler and not being used anywhere in codes, so remove them.
In docParser, actually, we are overloading three parent methods startElement(), endElement() and characters(), so I rename back three of these method names for example from start() to startElement() which makes it simpler to read.
make other improvements in codes.
Someone else will have to review this. Sorry. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- python/generator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/generator.py b/python/generator.py index 7ccd471..4b62f83 100755 --- a/python/generator.py +++ b/python/generator.py @@ -771,6 +771,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref,api_xml)) data = f.read() + f.close() parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() @@ -778,6 +779,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref,"..","docs",api_xml)) data = f.read() + f.close() parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() @@ -795,6 +797,7 @@ def buildStubs(module, stubs_buiding_debug = False, xml_parsing_debug = False): try: f = open(os.path.join(srcPref, override_api_xml)) data = f.read() + f.close() parser = getparser(xml_parsing_debug) parser.feed(data) parser.close() -- 1.7.11.2
On 02/28/2013 03:03 AM, Guannan Ren wrote:
--- python/generator.py | 3 +++ 1 file changed, 3 insertions(+)
At first, I was worried that this is a leak in the generated code. But it looks like it is only a leak in the generator, and that the generated code is safe. So it's not an essential bug fix, and more of a judgment call on whether to include in 1.0.3. At any rate, the fix appears obvious enough that I'm fine giving: ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/01/2013 06:26 AM, Eric Blake wrote:
On 02/28/2013 03:03 AM, Guannan Ren wrote:
--- python/generator.py | 3 +++ 1 file changed, 3 insertions(+) At first, I was worried that this is a leak in the generated code. But it looks like it is only a leak in the generator, and that the generated code is safe. So it's not an essential bug fix, and more of a judgment call on whether to include in 1.0.3. At any rate, the fix appears obvious enough that I'm fine giving:
ACK.
Thanks for this review. I pushed these two ack'ed patches before sending out another bug-fixing patch about generator.py. Guannan
participants (2)
-
Eric Blake -
Guannan Ren