diff mbox series

riscv: Add header include guards to insn.h

Message ID 20230129094242.282620-1-liaochang1@huawei.com (mailing list archive)
State Accepted
Commit 8ac6e619d9d51b3eb5bae817db8aa94e780a0db4
Delegated to: Palmer Dabbelt
Headers show
Series riscv: Add header include guards to insn.h | expand

Checks

Context Check Description
conchuod/cover_letter success Single patches do not need cover letters
conchuod/tree_selection success Guessed tree name to be for-next
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 13 and now 13
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 0 this patch: 0
conchuod/alphanumeric_selects success Out of order selects before the patch: 57 and now 57
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 2 this patch: 2
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 13 lines checked
conchuod/source_inline success Was 0 now: 0
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success Fixes tag looks correct
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Liao, Chang Jan. 29, 2023, 9:42 a.m. UTC
Add header include guards to insn.h to prevent repeating declaration of
any identifiers in insn.h.

Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB")
Signed-off-by: Liao Chang <liaochang1@huawei.com>
---
 arch/riscv/include/asm/insn.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Andrew Jones Jan. 30, 2023, 7:35 a.m. UTC | #1
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.
> 
> Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB")
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
>  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
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Conor Dooley Jan. 30, 2023, 2:53 p.m. UTC | #2
Hey!

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?

> 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@microchip.com>

> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
>  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
> 
>
Liao, Chang Feb. 1, 2023, 9:37 a.m. UTC | #3
Hi, Conor

在 2023/1/30 22:53, Conor Dooley 写道:
> Hey!
> 
> 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.

#!/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@microchip.com>
> 
>> Signed-off-by: Liao Chang <liaochang1@huawei.com>
>> ---
>>  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
>>
>>
Conor Dooley Feb. 1, 2023, 9:42 a.m. UTC | #4
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?

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@microchip.com>
> > 
> >> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> >> ---
> >>  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
>
Liao, Chang Feb. 2, 2023, 12:52 a.m. UTC | #5
在 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@microchip.com>
>>>
>>>> Signed-off-by: Liao Chang <liaochang1@huawei.com>
>>>> ---
>>>>  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
>>
Joe Perches Feb. 2, 2023, 3:33 a.m. UTC | #6
On Thu, 2023-02-02 at 08:52 +0800, liaochang (A) wrote:
> 
> 在 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?

Don't see why not.
Perhaps submit it as a real patch to Andrew Morton <akpm@linux-foundation.org>

Trivial notes below:

> > > #!/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.

Not illegal, invalid

> > >         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";

here too

There are also uses like #if !defined(FOO)
There can be spaces before and after the #

It might also be useful to have a check for missing header guards.
Liao, Chang Feb. 2, 2023, 11:36 a.m. UTC | #7
在 2023/2/2 11:33, Joe Perches 写道:
> On Thu, 2023-02-02 at 08:52 +0800, liaochang (A) wrote:
>>
>> 在 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?
> 
> Don't see why not.
> Perhaps submit it as a real patch to Andrew Morton <akpm@linux-foundation.org>
> 
> Trivial notes below:
> 
>>>> #!/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.
> 
> Not illegal, invalid
> 
>>>>         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";
> 
> here too
> 
> There are also uses like #if !defined(FOO)
> There can be spaces before and after the #
> 
> It might also be useful to have a check for missing header guards.

OK, i will use a stricter pattern in the real patch.

Thanks.

> 
> 
> 
>
patchwork-bot+linux-riscv@kernel.org Feb. 22, 2023, 3 p.m. UTC | #8
Hello:

This patch was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Sun, 29 Jan 2023 17:42:42 +0800 you wrote:
> Add header include guards to insn.h to prevent repeating declaration of
> any identifiers in insn.h.
> 
> Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB")
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> ---
>  arch/riscv/include/asm/insn.h | 4 ++++
>  1 file changed, 4 insertions(+)

Here is the summary with links:
  - riscv: Add header include guards to insn.h
    https://git.kernel.org/riscv/c/8ac6e619d9d5

You are awesome, thank you!
diff mbox series

Patch

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 */