Re: [PATCH v2 10/11] tools/kvm_stat: sort '-f help' output

Janosch Frank
2017-12-11T12:21:11Z

[-- Attachment #1.1: Type: text/plain, Size: 1968 bytes --]

On 11.12.2017 12:25, Stefan Raspl wrote:
> From: Stefan Raspl <stefan.raspl@de.ibm.com>
>
> Sort the fields returned by specifying '-f help' on the command line.
> While at it, simplify the code a bit, indent the output and eliminate an
> extra blank line at the beginning.
>
> Signed-off-by: Stefan Raspl <raspl@linux.vnet.ibm.com>
> ---
> tools/kvm/kvm_stat/kvm_stat | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/tools/kvm/kvm_stat/kvm_stat b/tools/kvm/kvm_stat/kvm_stat
> index 42c34b8818f7..929c8379d82a 100755
> --- a/tools/kvm/kvm_stat/kvm_stat
> +++ b/tools/kvm/kvm_stat/kvm_stat
> @@ -33,6 +33,7 @@ import resource
> import struct
> import re
> import subprocess
> +from sets import Set

What's the reason for this import, shouldn't set be built in from at
least 2.7 on? It even seems to be 2.4. The module should be deprecated
from 2.6 on.

> from collections import defaultdict
>
> VMX_EXIT_REASONS = {
> @@ -1572,17 +1573,14 @@ def main():
>
> stats = Stats(options)
>
> - if options.fields == "help":
> + if options.fields == 'help':
> stats.fields_filter = None
> - event_list = "\n"
> - s = stats.get()
> - for key in s.keys():
> - if key.find('(') != -1:
> - key = key[0:key.find('(')]
> - if event_list.find('\n' + key + '\n') == -1:
> - event_list += key + '\n'
> - sys.stdout.write(event_list)
> - return ""
> + event_list = []
> + for key in stats.get().keys():
> + event_list.append(key.split('(', 1)[0])

Definitely better.

> + event_list.sort()
> + sys.stdout.write(' ' + '\n '.join(sorted(Set(event_list))) + '\n')

You just sorted the list, why do you need a set and why do you sort it
again?

> + sys.exit(0)
>
> if options.log:
> log(stats)
>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]