This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Friday 27 December 2013 10:04:45 Siddhesh Poyarekar wrote: largely looks good. mostly nits below. when it comes to declaring coding style, i'm not sure the GNU project has one already for python (since they try to push Guile/scheme). GDB has been growing a python code base, but a scan of that tree doesn't turn up anything either :(. looking around the glibc tree, i think adding a section to manual/maint.texi would be the best bet. > --- a/scripts/bench.pl > +++ /dev/null since we're not meant to review this, the -D flag would help > --- /dev/null > +++ b/scripts/bench.py > @@ -0,0 +1,305 @@ > +#!/usr/bin/python > +# Copyright (C) 2013 Free Software Foundation, Inc. > +# This file is part of the GNU C Library. > +# > +# The GNU C Library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > +# > +# The GNU C Library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public > +# License along with the GNU C Library; if not, see > +# <http://www.gnu.org/licenses/>. > +"""Benchmark program generator script prefer a newline above that docstring > +def gen_source(func, directives, all_vals): > + """Generate source for the function > + > + Generate the C source for the function from the values and > + directives. > + > + Args: > + func: The function name > + directives: A dictionary of directives applicable to this function > + all_vals: A dictionary input values > + > + """ no need for that trailing blank line in the docstring. goes for all funcs in this file that i can see. > + for header in directives['include-sources']: > + print('#include \"%s\"' % header) are those \ still needed ? > + for k, i in zip(all_vals.keys(), itertools.count()): > + vals = all_vals[k] could do: for (k, v), i in zip(all_vals.items(), itertools.count()): > + # Members for the variants structure list that we will > + # print later. > + variants.append(' {\"%s(%s)\", %d, in%d},' % (func, k, len(vals), same here -- are those \ still needed ? > + directives = {'name': '', > + 'args': [], > + 'includes': [], > + 'include-sources': [], > + 'ret': ''} in cases like this, it's neater to uncuddle entirely: directives = { 'name': '', 'args': [], 'includes': [], 'include-sources': [], 'ret': '', } (i use tabs just because it's easier to type) > + for line in f: > + line = line.rstrip() > + > + # Look for directives and parse it if found. > + if line.startswith('##'): > + try: > + d_name, d_val = line[2:].split(':', 1) > + d_name = d_name.strip() > + d_val = d_val.strip() > + directives[d_name] = _process_directive(d_name, > + except (IndexError, KeyError): > + die('Invalid directive: %s' % line[2:]) > + > + # Skip blank lines and comments. > + line = line.split('#', 1)[0].rstrip() > + if not line: > + continue do you really need that initial rstrip() at the top of this loop ? the name/val parsing calls strip themselves, and this final line parser calls rstrip() too. > +def die(msg): > + """Exit with an error > + > + Prints an error message to the standard error stream and exits with > + a non-zero status. > + > + Args: > + msg: The error message to print to standard error. > + > + """ > + sys.stderr.write('%s\n' % msg) alternative: print('%s\n' % msg, file=sys.stderr) > +def main(args): > + """Main function > + > + Use the first command line argument as function name and parse its > + input file to generate C source that calls the function repeatedly > + for the input. > + > + Args: > + args: The command line arguments with the program name dropped. > + > + Returns: > + 1 on error and 0 on success. the os module provides EX_* constants ... > + if len(args) != 1: > + print('Usage: %s <function>' % sys.argv[0]) > + return 1 ... os.EX_USAGE > + directives, all_vals = parse_file(args[0]) > + gen_source(args[0], directives, all_vals) > + return 0 ... os.EX_OK > --- /dev/null > +++ b/scripts/pylintrc any reason for that vs generating a default one and tweaking it ? see pylint's --generate-rcfile option. would be helpful i think to add a scripts/pylint wrapper: #!/bin/sh exec pylint --rcfile "${0%/*}/pylintrc" "$@" that way people can do: ./scripts/pylint */*.py -mike
Attachment:
signature.asc
Description: This is a digitally signed message part.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |