Re: [cocci] [PATCH 2/2] scripts: coccicheck: Separate spatch stdout and stderr

[Anton Eliasson]

On 11/10/2023 15.46, Julia Lawall wrote:
> On Wed, 11 Oct 2023, Anton Eliasson wrote:
>
> > On 10/10/2023 18.11, Julia Lawall wrote:
> > > On Tue, 10 Oct 2023, Anton Eliasson wrote:
> > >
> > > > On 07/10/2023 21.41, Julia Lawall wrote:
> > > > > On Tue, 3 Oct 2023, Anton Eliasson wrote:
> > > > >
> > > > > > This helps automating coccicheck runs by discarding stderr and only
> > > > > > looking at the output of stdout. In report mode the only remaining
> > > > > > output on stdout is the initial "Please check for false positives"
> > > > > > message followed by each spatch warning found.
> > > > > What is getting dropped is the spatch command lines indicating the
> > > > > semantic patch.  Is this desirable?
> > > > >
> > > > > julia
> > > > It's not ideal but it's the best compromise that I have found. The problem
> > > > I'm
> > > > trying to solve is to be able to diff the output of two coccicheck runs
> > > > and
> > > > notify the developer if any new warnings were introduced. That requires
> > > > the
> > > > output to be stable. spatch is always invoked for each cocci file in the
> > > > same
> > > > order. However, the output from each spatch invocation is not stable as it
> > > > examines each source file in an arbitrary order.
> > > >
> > > > My workaround is to sort the output before diffing. The line-by-line
> > > > sorted
> > > > output only makes sense if the input is one line per warning found and
> > > > that is
> > > > why I try to discard all output except the single line per spatch warning.
> > > > While the terse output doesn't tell which semantic patch file generated
> > > > the
> > > > warning, it does usually contain the offending file, line number and a
> > > > summary
> > > > of the issue.
> > > Why does the command line pose a problem for sorting?
> > >
> > > julia
> > You're right. I was overthinking it. Since the sorted command lines will be
> > common for the two runs they will disappear after diffing.
> >
> > So at this point I don't have any need for this patch. I'll reach out to you
> > again if it turns out to be an issue after we have gotten the continuous
> > integration check in place. Thanks for the feedback and I'm sorry about the
> > noise.
> OK, thanks for the discussion.  I was also thinking about whether it could
> be possible to make the output always come out in the same order, based on
> the name of the analyzed file.  Maybe it is possible.
>
> julia

Yes, that might help to preserve some context. The difference between 
two runs could be presented as a unified diff instead of just lines 
added. That would probably contain the command line invoking the 
semantic patch that caused the warning, unless there are too many 
pre-existent warnings.


Anton

> > Anton
> >
> > > > Anton
> > > > > > Signed-off-by: Anton Eliasson <anton.eliasson@xxxxxxxx>
> > > > > > ---
> > > > > >     scripts/coccicheck | 4 ++--
> > > > > >     1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/scripts/coccicheck b/scripts/coccicheck
> > > > > > index 95a312730e98..7e7c44125f47 100755
> > > > > > --- a/scripts/coccicheck
> > > > > > +++ b/scripts/coccicheck
> > > > > > @@ -146,8 +146,8 @@ run_cmd_parmap() {
> > > > > >                     echo $@>>$DEBUG_FILE
> > > > > >                     $@ 2>>$DEBUG_FILE
> > > > > >             else
> > > > > > -                echo $@
> > > > > > -                $@ 2>&1
> > > > > > +                echo $@ >&2
> > > > > > +                $@
> > > > > >     	fi
> > > > > >
> > > > > >     	err=$?
> > > > > >
> > > > > > --
> > > > > > 2.30.2