| Message ID | 1457710283-12722-1-git-send-email-tvrtko.ursulin@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/03/16 15:31, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@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@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? Regards, Tvrtko
On Thu, 17 Mar 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: > [ text/plain ] > > > On 11/03/16 15:31, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@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@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. > > Regards, > > Tvrtko
On 17/03/16 11:27, Jani Nikula wrote: > On Thu, 17 Mar 2016, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote: >> [ text/plain ] >> >> >> On 11/03/16 15:31, Tvrtko Ursulin wrote: >>> From: Tvrtko Ursulin <tvrtko.ursulin@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@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.
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