[PATCH v2 0/4] HMP: hxtool sanity checks

[PATCH v2 0/4] HMP: hxtool sanity checks

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

Hi,
  This adds a couple of sanity checks to hxtool to try and catch the
  type of mistake that Markus' patch fixes.
  (Included Markus' patch since otherwise the script correctly fails)

Dave

v2:
  Add brackets on the .name regexp
  Use snake_case for names.

Dr. David Alan Gilbert (3):
  hmp*hx: Move info docs
  hxtool: Split srst/erst add checks
  hxtool: Error on missing docs

Markus Armbruster (1):
  hmp-commands-info.hx: Move definition of "info accel"

 hmp-commands-info.hx | 33 +++++++++++++--------------------
 hmp-commands.hx      | 27 +++++++++++++++++----------
 scripts/hxtool       | 38 +++++++++++++++++++++++++++++++++++---
 3 files changed, 65 insertions(+), 33 deletions(-)

-- 
2.52.0

[PATCH v2 1/4] hmp-commands-info.hx: Move definition of "info accel"

[dave]

From: Markus Armbruster <armbru@redhat.com>

Commit c10eb740108 (accel/system: Add 'info accel' on human monitor)
inserted "info accel" in the middle of "info sync-profile".  Move it
behind "info sync-profile".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 hmp-commands-info.hx | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 41674dcbe1..74c741f80e 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -267,18 +267,6 @@ ERST
         .cmd        = hmp_info_sync_profile,
     },
 
-    {
-        .name       = "accel",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show accelerator statistics",
-    },
-
-SRST
-  ``info accel``
-    Show accelerator statistics.
-ERST
-
 SRST
   ``info sync-profile [-m|-n]`` [*max*]
     Show synchronization profiling info, up to *max* entries (default: 10),
@@ -294,6 +282,18 @@ SRST
     being coalesced.
 ERST
 
+    {
+        .name       = "accel",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show accelerator statistics",
+    },
+
+SRST
+  ``info accel``
+    Show accelerator statistics.
+ERST
+
     {
         .name       = "kvm",
         .args_type  = "",
-- 
2.52.0

[PATCH v2 2/4] hmp*hx: Move info docs

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

Move the docs for the info subcommand from the separate hx
into the top level file next to the 'info' command itself.
That makes every command in the top level file have a RST section.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hmp-commands-info.hx |  9 +--------
 hmp-commands.hx      | 27 +++++++++++++++++----------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 74c741f80e..964eed004c 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -8,14 +8,7 @@ HXCOMM
 HXCOMM In this file, generally SRST fragments should have two extra
 HXCOMM spaces of indent, so that the documentation list item for "info foo"
 HXCOMM appears inside the documentation list item for the top level
-HXCOMM "info" documentation entry. The exception is the first SRST
-HXCOMM fragment that defines that top level entry.
-
-SRST
-``info`` *subcommand*
-  Show various information about the system state.
-
-ERST
+HXCOMM "info" documentation entry.
 
     {
         .name       = "version",
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 5cc4788f12..f4a6eeda93 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1814,16 +1814,6 @@ SRST
   command.
 ERST
 
-    {
-        .name       = "info",
-        .args_type  = "item:s?",
-        .params     = "[subcommand]",
-        .help       = "show various information about the system state",
-        .cmd        = hmp_info_help,
-        .sub_table  = hmp_info_cmds,
-        .flags      = "p",
-    },
-
 #if defined(CONFIG_FDT)
     {
         .name       = "dumpdtb",
@@ -1867,3 +1857,20 @@ SRST
   List event channels in the guest
 ERST
 #endif
+
+HXCOMM *** MUST BE LAST ENTRY **
+    {
+        .name       = "info",
+        .args_type  = "item:s?",
+        .params     = "[subcommand]",
+        .help       = "show various information about the system state",
+        .cmd        = hmp_info_help,
+        .sub_table  = hmp_info_cmds,
+        .flags      = "p",
+    },
+
+SRST
+``info`` *subcommand*
+  Show various information about the system state.
+ERST
+HXCOMM *** MUST BE LAST ENTRY **
-- 
2.52.0

[PATCH v2 3/4] hxtool: Split srst/erst add checks

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

Split the SRST/ERST case and add some checks.
This is mainly to make it easier to add some checks in following
patches.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 scripts/hxtool | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/scripts/hxtool b/scripts/hxtool
index 80516b9437..51dc841479 100755
--- a/scripts/hxtool
+++ b/scripts/hxtool
@@ -2,15 +2,29 @@
 
 hxtoh()
 {
-    flag=1
+    in_rst=0
     while read -r str; do
         case $str in
             HXCOMM*)
             ;;
-            SRST*|ERST*) flag=$(($flag^1))
+            SRST*)
+              if [ $in_rst -eq 1 ]
+              then
+                echo "Error: SRST inside another RST" >&2
+                exit 1
+              fi
+              in_rst=1
+            ;;
+            ERST*)
+              if [ $in_rst -eq 0 ]
+              then
+                echo "Error: ERST already outside RST" >&2
+                exit 1
+              fi
+              in_rst=0
             ;;
             *)
-            test $flag -eq 1 && printf "%s\n" "$str"
+            test $in_rst -eq 0 && printf "%s\n" "$str"
             ;;
         esac
     done
-- 
2.52.0

[PATCH v2 4/4] hxtool: Error on missing docs

[dave]

From: "Dr. David Alan Gilbert" <dave@treblig.org>

Error if a '.name' is seen after another '.name' without an intervening
SRST, this normally indicates missing or misplaced docs.

We can't check DEF (as used in command line options) because those
often have multiple DEF per doc.

Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
---
 scripts/hxtool | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/scripts/hxtool b/scripts/hxtool
index 51dc841479..ee98fb9c09 100755
--- a/scripts/hxtool
+++ b/scripts/hxtool
@@ -1,8 +1,14 @@
 #!/bin/sh
 
+print_if_not_rst()
+{
+    test $in_rst -eq 0 && printf "%s\n" "$str"
+}
 hxtoh()
 {
     in_rst=0
+    # .name for HMP
+    seen_name=0
     while read -r str; do
         case $str in
             HXCOMM*)
@@ -13,6 +19,8 @@ hxtoh()
                 echo "Error: SRST inside another RST" >&2
                 exit 1
               fi
+              # consume the name
+              seen_name=0
               in_rst=1
             ;;
             ERST*)
@@ -23,6 +31,16 @@ hxtoh()
               fi
               in_rst=0
             ;;
+            # Note the space at the start - we need to exclude something.name
+            ( .name*)
+              if [ $seen_name -eq 1 ]
+              then
+                echo "Error: Seen another .name, maybe missing docs?" >&2
+                exit 1
+              fi
+              seen_name=1
+              print_if_not_rst
+            ;;
             *)
             test $in_rst -eq 0 && printf "%s\n" "$str"
             ;;
-- 
2.52.0

Re: [PATCH v2 3/4] hxtool: Split srst/erst add checks

[Thomas Huth]

On 16/01/2026 01.50, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> Split the SRST/ERST case and add some checks.
> This is mainly to make it easier to add some checks in following
> patches.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
>   scripts/hxtool | 20 +++++++++++++++++---
>   1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/hxtool b/scripts/hxtool
> index 80516b9437..51dc841479 100755
> --- a/scripts/hxtool
> +++ b/scripts/hxtool
> @@ -2,15 +2,29 @@
>   
>   hxtoh()
>   {
> -    flag=1
> +    in_rst=0
>       while read -r str; do
>           case $str in
>               HXCOMM*)
>               ;;
> -            SRST*|ERST*) flag=$(($flag^1))
> +            SRST*)
> +              if [ $in_rst -eq 1 ]
> +              then
> +                echo "Error: SRST inside another RST" >&2
> +                exit 1
> +              fi
> +              in_rst=1
> +            ;;
> +            ERST*)
> +              if [ $in_rst -eq 0 ]
> +              then
> +                echo "Error: ERST already outside RST" >&2
> +                exit 1
> +              fi
> +              in_rst=0
>               ;;
>               *)
> -            test $flag -eq 1 && printf "%s\n" "$str"
> +            test $in_rst -eq 0 && printf "%s\n" "$str"
>               ;;
>           esac
>       done

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 4/4] hxtool: Error on missing docs

[Thomas Huth]

On 16/01/2026 01.50, dave@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <dave@treblig.org>
> 
> Error if a '.name' is seen after another '.name' without an intervening
> SRST, this normally indicates missing or misplaced docs.
> 
> We can't check DEF (as used in command line options) because those
> often have multiple DEF per doc.
> 
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
>   scripts/hxtool | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/scripts/hxtool b/scripts/hxtool
> index 51dc841479..ee98fb9c09 100755
> --- a/scripts/hxtool
> +++ b/scripts/hxtool
> @@ -1,8 +1,14 @@
>   #!/bin/sh
>   
> +print_if_not_rst()
> +{
> +    test $in_rst -eq 0 && printf "%s\n" "$str"
> +}

Add an empty line after this function?

>   hxtoh()
>   {
>       in_rst=0
> +    # .name for HMP
> +    seen_name=0
>       while read -r str; do
>           case $str in
>               HXCOMM*)
> @@ -13,6 +19,8 @@ hxtoh()
>                   echo "Error: SRST inside another RST" >&2
>                   exit 1
>                 fi
> +              # consume the name
> +              seen_name=0
>                 in_rst=1
>               ;;
>               ERST*)
> @@ -23,6 +31,16 @@ hxtoh()
>                 fi
>                 in_rst=0
>               ;;
> +            # Note the space at the start - we need to exclude something.name
> +            ( .name*)
> +              if [ $seen_name -eq 1 ]
> +              then
> +                echo "Error: Seen another .name, maybe missing docs?" >&2
> +                exit 1
> +              fi
> +              seen_name=1
> +              print_if_not_rst
> +            ;;
>               *)
>               test $in_rst -eq 0 && printf "%s\n" "$str"

I think this line could be replaced with print_if_not_rst now?

  Thomas

Re: [PATCH v2 1/4] hmp-commands-info.hx: Move definition of "info accel"

[Philippe Mathieu-Daudé]

On 16/1/26 01:50, dave@treblig.org wrote:
> From: Markus Armbruster <armbru@redhat.com>
> 
> Commit c10eb740108 (accel/system: Add 'info accel' on human monitor)
> inserted "info accel" in the middle of "info sync-profile".  Move it
> behind "info sync-profile".

Oops...

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
>   hmp-commands-info.hx | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)

Re: [PATCH v2 2/4] hmp*hx: Move info docs

[Markus Armbruster]

dave@treblig.org writes:

> From: "Dr. David Alan Gilbert" <dave@treblig.org>
>
> Move the docs for the info subcommand from the separate hx
> into the top level file next to the 'info' command itself.
> That makes every command in the top level file have a RST section.

Could mention:

  Generated HTML changes slightly.  The difference is basically
  invisible.

> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v2 3/4] hxtool: Split srst/erst add checks

[Markus Armbruster]

dave@treblig.org writes:

> From: "Dr. David Alan Gilbert" <dave@treblig.org>
>
> Split the SRST/ERST case and add some checks.
> This is mainly to make it easier to add some checks in following
> patches.
>
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v2 4/4] hxtool: Error on missing docs

[Markus Armbruster]

dave@treblig.org writes:

> From: "Dr. David Alan Gilbert" <dave@treblig.org>
>
> Error if a '.name' is seen after another '.name' without an intervening
> SRST, this normally indicates missing or misplaced docs.
>
> We can't check DEF (as used in command line options) because those
> often have multiple DEF per doc.
>
> Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
> ---
>  scripts/hxtool | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/scripts/hxtool b/scripts/hxtool
> index 51dc841479..ee98fb9c09 100755
> --- a/scripts/hxtool
> +++ b/scripts/hxtool
> @@ -1,8 +1,14 @@
>  #!/bin/sh
>  
> +print_if_not_rst()
> +{
> +    test $in_rst -eq 0 && printf "%s\n" "$str"
> +}
>  hxtoh()
>  {
>      in_rst=0
> +    # .name for HMP
> +    seen_name=0
>      while read -r str; do
>          case $str in
>              HXCOMM*)
> @@ -13,6 +19,8 @@ hxtoh()
>                  echo "Error: SRST inside another RST" >&2
>                  exit 1
>                fi
> +              # consume the name
> +              seen_name=0
>                in_rst=1
>              ;;
>              ERST*)
> @@ -23,6 +31,16 @@ hxtoh()
>                fi
>                in_rst=0
>              ;;
> +            # Note the space at the start - we need to exclude something.name
> +            ( .name*)

TIL about the optional left parenthesis here.

> +              if [ $seen_name -eq 1 ]
> +              then
> +                echo "Error: Seen another .name, maybe missing docs?" >&2
> +                exit 1
> +              fi
> +              seen_name=1
> +              print_if_not_rst
> +            ;;
>              *)
>              test $in_rst -eq 0 && printf "%s\n" "$str"
>              ;;

Reviewed-by: Markus Armbruster <armbru@redhat.com>