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

[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?

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
>

Attachment: signature.asc
Description: PGP signature