[Intel-gfx] [PATCH v2] dim: Avoid false positives with BUG detection

[Dave Gordon]

On 17/03/16 11:27, Jani Nikula wrote:
> On Thu, 17 Mar 2016, Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com> wrote:
>> [ text/plain ]
>>
>>
>> On 11/03/16 15:31, Tvrtko Ursulin wrote:
>>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>>
>>> Makes sure that the diff line adding the BUG is not immediately
>>> preceded by the diff line removing the BUG. Or in other words,
>>> avoids false positives when existing BUG is edited.
>>>
>>> v2: Sent the incomplete version out...
>>>
>>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>> ---
>>>    dim | 3 ++-
>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/dim b/dim
>>> index 1e7622a1e902..5392c64bf5b4 100755
>>> --- a/dim
>>> +++ b/dim
>>> @@ -691,7 +691,8 @@ function shell_checkpatch
>>>    	local cmd=$1
>>>
>>>    	$cmd | scripts/checkpatch.pl -q --strict - || true
>>> -	if $cmd | grep '^\+.*\WBUG' > /dev/null; then
>>> +	local bug_lines=$("$cmd" | grep -m 1 -B 1 '^\+.*\WBUG' | grep -c '^[+-].*\WBUG')
>>> +	if test "$bug_lines" -eq 1; then
>>>    		warn_or_fail "New BUG macro added"
>>>    	fi
>>>    	$cmd | grep '^\+.*drm_i915_private_t' > /dev/null && echo "WARNING: New drm_i915_private_t added" || true
>>>
>>
>> Are we interested in this?
>
> Yes. Sorry, I've updated these lines so you'll need to rebase, I'm
> afraid.
>
> I'd also be fine with comparing the results of $(grep -c '^\+.*\WBUG)
> and $(grep -c '^\-.*\WBUG) and complaining if there are more +'s than
> -'s. But your stricter version is fine too.
>
> BR,
> Jani.

The version above doesn't really avoid all the issues, though; for 
example if the edited BUG spans multiple lines, or if the pattern 
spotted (first added line containing a word beginning with BUG) is in a 
comment or string, or what's actually happened is that some code has 
been moved from before an existing BUG() to after or vice-versa, but 
diff thinks you've deleted the old BUG() and added a new one :(

.Dave.