On Thu, 25 Jul 2024 at 06:55, Yao Xingtao via <qemu-devel@nongnu.org> wrote:
>
> This is the semantic patch from commit 7b3e371526 "cxl/mailbox: make
> range overlap check more readable"
>
> Signed-off-by: Yao Xingtao <yaoxt.f...@fujitsu.com>
> ---
>  scripts/coccinelle/range.cocci | 49 ++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>  create mode 100644 scripts/coccinelle/range.cocci
>
> diff --git a/scripts/coccinelle/range.cocci b/scripts/coccinelle/range.cocci
> new file mode 100644
> index 000000000000..21b07945ccb2
> --- /dev/null
> +++ b/scripts/coccinelle/range.cocci
> @@ -0,0 +1,49 @@
> +/*
> +  Usage:
> +
> +    spatch \
> +           --macro-file scripts/cocci-macro-file.h \
> +           --sp-file scripts/coccinelle/range.cocci \
> +           --keep-comments \
> +           --in-place \
> +           --dir .
> +
> +  Description:
> +    Find out the range overlap check and use ranges_overlap() instead.
> +
> +  Note:
> +    This pattern cannot accurately match the region overlap check, and you
> +    need to manually delete the use cases that do not meet the conditions.
> +
> +    In addition, the parameters of ranges_overlap() may be filled 
> incorrectly,
> +    and some use cases may be better to use range_overlaps_range().

I think these restrictions mean that we should do one
of two things:
 (1) rewrite this as a Coccinelle script that just prints
     out the places where it found matches (i.e. a "grep"
     type operation, not a search-and-replace), so the
     user can then go and investigate them and do the
     range_overlap they want to do
 (2) fix the problems so that you really can apply it to
     the source tree and get correct fixes

The ideal would be (2) -- the ideal flow for coccinelle
driven patches is that you can review the coccinelle
script to check for things like off-by-one errors, and
then can trust all the changes it makes. Otherwise
reviewers need to carefully scrutinize each source
change individually.

It's the off-by-one issue that really makes me think
that (2) would be preferable -- it would otherwise
be I think quite easy to accidentally rewrite a correct
range check into one that's off-by-one and not notice.

thanks
-- PMM

Reply via email to