[PATCH] devtool: fix falsely reporting from checkpatch
David Marchand
david.marchand at redhat.com
Tue Jan 28 16:02:27 CET 2025
Hi,
On Mon, Jan 20, 2025 at 12:27 PM WanRenyong <wanry at yunsilicon.com> wrote:
>
> When executes the check_packed_attributes function in checkpatch,
> if __rte_packed_begin or __rte_packed_end appear in the context
> of a patch file, there may be a situation where the counts of
> __rte_packed_begin and __rte_packed_end do not match, causing
> checkpatch to return a failure.
> This patch fixes this issue by only counting the lines in the
> patch file that start with a + and include either
> __rte_packed_begin or __rte_packed_end.
>
> Signed-off-by: WanRenyong <wanry at yunsilicon.com>
Thanks for proposing this fix.
When sending such fixes, don't forget to add a Fixes: and Cc:
author/maintainers by using --cc-cmd devtools/get-maintainers.sh
Adding Akhil, André and Thomas in the loop.
I also had some concern about the check:
https://inbox.dpdk.org/dev/CAJFAV8w=s1L-WYk+Qv-B+Mn6eAwKrB=GTz6hU--ZoLrJsz7=DQ@mail.gmail.com/
But I merged the check untouched as nobody else seemed to object.
Checkpatch warnings are known to have false positive and such false
positives are "filtered" by subtree maintainers.
> ---
> devtools/checkpatches.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 003bb49e04..2e228b7f92 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -384,8 +384,8 @@ check_packed_attributes() { # <patch>
> res=1
> fi
>
> - begin_count=$(grep '__rte_packed_begin' "$1" | wc -l)
> - end_count=$(grep '__rte_packed_end' "$1" | wc -l)
> + begin_count=$(grep -E '^\+.*__rte_packed_begin' "$1" | wc -l)
> + end_count=$(grep -E '^\+.*__rte_packed_end' "$1" | wc -l)
> if [ $begin_count != $end_count ]; then
> echo "__rte_packed_begin and __rte_packed_end should always be used in pairs."
> res=1
I don't think this change is any better.
There is a good chance that a patch touching just a first line of a
structure definition won't come along a line touching the last line of
the struct.
My suggestion (if we want to avoid those non useful warning) would be
to just remove the counting stuff in the check.
Opinions?
--
David Marchand
More information about the dev
mailing list