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

[Simon Glass]

Hi Doug,

On Thu, Mar 21, 2013 at 9:35 AM, Doug Anderson <dianders at chromium.org> wrote:
> 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.

Yes - I'll add them to the commit message.

>
>
>> @@ -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?

Yes, we are well past the point of needing something like that, so will add it.

>
>
>>          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?

Yes - we don't currently use the return value. But I will fix it.

Regards,
Simon

>
>
> -Doug