On Thu, 25 Jul 2024 at 06:55, Yao Xingtao via <[email protected]> wrote:
>
> This is the semantic patch from commit 7b3e371526 "cxl/mailbox: make
> range overlap check more readable"
>
> Signed-off-by: Yao Xingtao <[email protected]>
> ---
> 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
