Re: [Qemu-devel] [RFC PATCH v2 1/8] tracetool: Rewrite infrastructure as python modules

[Lluís Vilanova]

Alon Levy writes:

[...]
>> +
>> +
>> +_SCRIPT = ""

> I don't understand the point of this, why not use sys.argv[0] directly?

[...]
>> +def main(args):
>> +    global _SCRIPT
>> +    _SCRIPT = sys.argv[0]
>> +

Well, my idea is that it must work only with the arguments to 'main',
without depending on any other system state.

BTW, I fixed main to be "_SCRIPT = args[0]" instead of "sys.argv[0]" (otherwise
it makes no sense).


[...]
>> +def get_list(only_public = False):
>> +    """Get a list of (name, description) pairs."""
>> +    res = [("nop", "Tracing disabled.")]
>> +    for _, modname, _ in pkgutil.iter_modules(tracetool.backend.__path__):
>> +        module = tracetool.try_import("tracetool.backend." + modname)[1]
>> +

> Here module can be None if ImportError was raised in try_import.

>> +        public = getattr(module, "PUBLIC", False)

> public will be False


Not really; as it only tries to import files stemming from
'pkgutil.iter_modules', this should always succeed.

In any case, I put a guard just in case someone puts a non-module file there
(which should never happen).



>> +        if only_public and not public:
>> +            continue

> continue isn't reached if not only_public (default value)

Right, 'only_public' is only used sometimes (when handling "--list-backends"
cmdline option).


I'll send a new version once I put all the changes into my tree.


Thanks for all the catches,
    Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth