[U-Boot] [PATCH 1/8] patman: Fix up checkpatch parsing to deal with 'CHECK' lines

Doug Anderson dianders at chromium.org
Thu Mar 21 17:35:40 CET 2013


Simon,

On Wed, Mar 20, 2013 at 7:42 PM, Simon Glass <sjg at chromium.org> wrote:
> checkpatch has a new type of warning, a 'CHECK'. At present patman fails
> with these, which makes it less than useful.
>
> Add support for checks, making it backwards compatible with the old
> checkpatch.

There are also a few other minor fixups in this:
* Cleanup formatting of the CheckPatches() output.
* Fix erroneous "internal error" if multiple patches have warnings.
* Be more robust to new types of problems.


> @@ -64,10 +64,14 @@ def CheckPatch(fname, verbose=False):
>                  'msg': text message
>                  'file' : filename
>                  'line': line number
> +            error_count: Number of errors
> +            warning_count: Number of warnings
> +            check_count: Number of checks
>              lines: Number of lines
> +            stdout: Full output of checkpatch

nit: Right above this it says you're returning a 4-tuple.  That's no
longer true.  Could just remove the "4-".

optional: I've found that returning big tuples like this is
problematic.  Whenever you change the number of things returned you've
got to modify any callers that call like:

a, b, c, d = CheckPatch(...)

to now be:

a, b, c, d, e = CheckPatch(...)

...or never use the above syntax and use:

result = CheckPatch(...)
blah = result[0]

Maybe use a namedtuple so that callers can use the result more cleanly?


>          if match:
>              error_count = int(match.group(1))
>              warning_count = int(match.group(2))
> -            lines = int(match.group(3))
> +            match_count = int(match.group(3))
> +            if len(match.groups()) == 4:
> +               check_count = match_count
> +               lines = int(match.group(4))

I'm confused about match_count here.  What is it supposed to contain?
I can't tell from the name of it.  It looks like a temporary variable
holding either check_count or lines.  ...but you forget to assign
"lines = match_count" in an "else" case so things are broken with old
versions of checkpatch, right?


-Doug


More information about the U-Boot mailing list