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

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

[Liao Chang]

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

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

[Liao Chang]

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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Andrew Jones]

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>

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

[Andrew Jones]

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>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1661 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1661 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[liaochang (A)]

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

-- 
BR,
Liao, Chang

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

[liaochang (A)]

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

-- 
BR,
Liao, Chang

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 4003 bytes --]

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
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4003 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[liaochang (A)]

在 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
>>

-- 
BR,
Liao, Chang

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

[liaochang (A)]

在 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
>>

-- 
BR,
Liao, Chang

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

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

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

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




_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[liaochang (A)]

在 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.

> 
> 
> 
> 

-- 
BR,
Liao, Chang

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

[liaochang (A)]

在 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.

> 
> 
> 
> 

-- 
BR,
Liao, Chang

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

[patchwork-bot+linux-riscv]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[patchwork-bot+linux-riscv]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv