Re: [PATCH] riscv: Add header include guards to insn.h

From: liaochang (A)
Date: Wed Feb 01 2023 - 19:52:50 EST




在 2023/2/1 17:42, Conor Dooley 写道:
> Hi,
>
> On Wed, Feb 01, 2023 at 05:37:24PM +0800, liaochang (A) wrote:
>> 在 2023/1/30 22:53, Conor Dooley 写道:
>>> On Sun, Jan 29, 2023 at 05:42:42PM +0800, Liao Chang wrote:
>>>> Add header include guards to insn.h to prevent repeating declaration of
>>>> any identifiers in insn.h.
>>>
>>> I'm curious, did you spot this "by hand" while doing other work, or do
>>> you have a tool that found it for you?
>>
>> I found this "by hand", inspired by scripts/checkdeclares.pl, i write a tiny tool
>> to analyse the missing header guards in header file.
>
> Ohh, cool! I'd love to add this one to the checks on patchwork so that
> we do not end up adding any more of these. If this is based on
> checkdeclares, is it GPLv2?

Definitely, it is GPLv2,i will appending licence identifier and copyright info later.

Hi,@Joe Perches, is it ok to integrate this tool to scripts?

Thanks.

>
> Thanks!
>
>>
>> #!/usr/bin/env perl
>> use strict;
>>
>> sub usage {
>> print "Usage: checkguards.pl file1.h ...\n";
>> print "Warn of missing header guards\n";
>> exit 1;
>> }
>>
>> if ($#ARGV < 0) {
>> usage();
>> }
>>
>> foreach my $file (@ARGV) {
>> open(my $f, '<', $file)
>> or die "Cannot open $file: $!.\n";
>>
>> my $scan_area = 1;
>> my $guards_warn = 0;
>>
>> # The lines of header file are divided into several areas as follows:
>> #
>> # ... area1 ...
>> # #ifndef _HEADER_GUARD
>> # ... area2 ...
>> # #define _HEADER_GUARD
>> # ... area3 ...
>> # #endif /* _HEADER_GUARD */
>> # ... area4 ...
>> # EOF
>> #
>> # If any statement is found in area1, area2, and area4, it
>> # throws a warning of illegal usage of header guard usage.
>> while (<$f>) {
>> if (m/^(.*);\s*$/o) {
>> if ($scan_area == 1 || $scan_area == 2 || $scan_area == 4) {
>> ++$guards_warn;
>> }
>> } elsif (m/^\s*(#ifndef\s+)[a-zA-Z0-9_]*_H[_]*\s*$/o) {
>> ++$scan_area;
>> } elsif (m/^\s*(#define\s+)[a-zA-Z0-9_]*_H[_]*\s*$/o) {
>> ++$scan_area;
>> } elsif (m/^\s*(#endif)\s*\/\*\s*[a-zA-Z0-9_]*_H[_]*\s*\*\/\s*$/o) {
>> ++$scan_area;
>> }
>> }
>>
>> close($f);
>>
>> if ($guards_warn) {
>> print "Illegal usage of header guard found in $file.\n";
>> }
>> }
>>
>> Thanks.
>>
>>>
>>>> Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB")
>>>
>>> Heh, I appreciate you going back to the file's original name to find the
>>> correct fixes tag!
>>> I figure that it's probably worth adding a fixes tag for the rename too,
>>> so that the stable bots don't get confused? That would be:
>>> Fixes: c9c1af3f186a ("RISC-V: rename parse_asm.h to insn.h")
>>>
>>> Probably overkill when you have Drew's already for something so trivial,
>>> but:
>>> Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>>>
>>>> Signed-off-by: Liao Chang <liaochang1@xxxxxxxxxx>
>>>> ---
>>>> arch/riscv/include/asm/insn.h | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
>>>> index 25ef9c0b19e7..22c7613bfda3 100644
>>>> --- a/arch/riscv/include/asm/insn.h
>>>> +++ b/arch/riscv/include/asm/insn.h
>>>> @@ -3,6 +3,9 @@
>>>> * Copyright (C) 2020 SiFive
>>>> */
>>>>
>>>> +#ifndef _ASM_RISCV_INSN_H
>>>> +#define _ASM_RISCV_INSN_H
>>>> +
>>>> #include <linux/bits.h>
>>>>
>>>> #define RV_INSN_FUNCT3_MASK GENMASK(14, 12)
>>>> @@ -365,3 +368,4 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 *utype_insn, u32 *itype
>>>> *utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1);
>>>> *itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);
>>>> }
>>>> +#endif /* _ASM_RISCV_INSN_H */
>>>> --
>>>> 2.25.1
>>>>
>>>>
>>
>> --
>> BR,
>> Liao, Chang
>>

--
BR,
Liao, Chang