[PATCH 2/5] arm64/sysreg: Enforce whole line match for closing blocks

James Clark posted 5 patches 11 months ago
There is a newer version of this series
[PATCH 2/5] arm64/sysreg: Enforce whole line match for closing blocks
Posted by James Clark 11 months ago
Match on the whole line to prevent matching on prefixes like "Endsysreg"
vs "EndsysregFields". This could potentially make the script go wrong
in weird ways so make it fall through to the fatal unhandled statement
catcher if it doesn't fully match the current block.

Signed-off-by: James Clark <james.clark@linaro.org>
---
 arch/arm64/tools/gen-sysreg.awk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
index 1a2afc9fdd42..7c7412adf90e 100755
--- a/arch/arm64/tools/gen-sysreg.awk
+++ b/arch/arm64/tools/gen-sysreg.awk
@@ -127,7 +127,7 @@ END {
 	next
 }
 
-/^EndSysregFields/ && block_current() == "SysregFields" {
+/^EndSysregFields$/ && block_current() == "SysregFields" {
 	if (next_bit > 0)
 		fatal("Unspecified bits in " reg)
 
@@ -177,7 +177,7 @@ END {
 	next
 }
 
-/^EndSysreg/ && block_current() == "Sysreg" {
+/^EndSysreg$/ && block_current() == "Sysreg" {
 	if (next_bit > 0)
 		fatal("Unspecified bits in " reg)
 
@@ -310,7 +310,7 @@ END {
 	next
 }
 
-/^EndEnum/ && block_current() == "Enum" {
+/^EndEnum$/ && block_current() == "Enum" {
 
 	field = null
 	msb = null
-- 
2.34.1
Re: [PATCH 2/5] arm64/sysreg: Enforce whole line match for closing blocks
Posted by Marc Zyngier 11 months ago
On Wed, 15 Jan 2025 13:42:54 +0000,
James Clark <james.clark@linaro.org> wrote:
> 
> Match on the whole line to prevent matching on prefixes like "Endsysreg"
> vs "EndsysregFields". This could potentially make the script go wrong
> in weird ways so make it fall through to the fatal unhandled statement
> catcher if it doesn't fully match the current block.
> 
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
>  arch/arm64/tools/gen-sysreg.awk | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
> index 1a2afc9fdd42..7c7412adf90e 100755
> --- a/arch/arm64/tools/gen-sysreg.awk
> +++ b/arch/arm64/tools/gen-sysreg.awk
> @@ -127,7 +127,7 @@ END {
>  	next
>  }
>  
> -/^EndSysregFields/ && block_current() == "SysregFields" {
> +/^EndSysregFields$/ && block_current() == "SysregFields" {

The problem with this sort of things is that it will now fail with
trailing spaces, which is both counter-intuitive and pretty hard to
spot.

Why don't you simply match the field number, like you do in patch 3?

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH 2/5] arm64/sysreg: Enforce whole line match for closing blocks
Posted by James Clark 11 months ago

On 15/01/2025 2:17 pm, Marc Zyngier wrote:
> On Wed, 15 Jan 2025 13:42:54 +0000,
> James Clark <james.clark@linaro.org> wrote:
>>
>> Match on the whole line to prevent matching on prefixes like "Endsysreg"
>> vs "EndsysregFields". This could potentially make the script go wrong
>> in weird ways so make it fall through to the fatal unhandled statement
>> catcher if it doesn't fully match the current block.
>>
>> Signed-off-by: James Clark <james.clark@linaro.org>
>> ---
>>   arch/arm64/tools/gen-sysreg.awk | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
>> index 1a2afc9fdd42..7c7412adf90e 100755
>> --- a/arch/arm64/tools/gen-sysreg.awk
>> +++ b/arch/arm64/tools/gen-sysreg.awk
>> @@ -127,7 +127,7 @@ END {
>>   	next
>>   }
>>   
>> -/^EndSysregFields/ && block_current() == "SysregFields" {
>> +/^EndSysregFields$/ && block_current() == "SysregFields" {
> 
> The problem with this sort of things is that it will now fail with
> trailing spaces, which is both counter-intuitive and pretty hard to
> spot.
> 
> Why don't you simply match the field number, like you do in patch 3?
> 
> 	M.
> 

The intention was to ensure that the end tokens were the only thing on 
the line (including whitespace). But yeah it is slightly inconsistent 
compared to start tokens because as you mention "$1 ==" allows both 
leading and trailing whitespace.

I'll change it to "$1 ==" for end tokens as well and then add 
expect_fields(1) which catches unexpected trailing stuff but has a 
better warning.

James
Re: [PATCH 2/5] arm64/sysreg: Enforce whole line match for closing blocks
Posted by Mark Brown 11 months ago
On Wed, Jan 15, 2025 at 01:42:54PM +0000, James Clark wrote:
> Match on the whole line to prevent matching on prefixes like "Endsysreg"
> vs "EndsysregFields". This could potentially make the script go wrong
> in weird ways so make it fall through to the fatal unhandled statement
> catcher if it doesn't fully match the current block.

Reviewed-by: Mark Brown <broonie@kernel.org>