thr3ads.net - Libguestfs - [Libguestfs] [PATCH v2 3/5] daemon/parted: simplify print_partition

If this information is useful, please help other people find it:
Share via:

Laszlo Ersek

2021-Nov-24 10:37 UTC

[Libguestfs] [PATCH v2 3/5] daemon/parted: simplify print_partition_table() prototype

Since commit 994ca1f8ebcc ("daemon: Reimplement
'part_get_mbr_part_type'
API in OCaml.", 2018-05-02), we've not had any calls to
print_partition_table() that would pass a "true" argument for the
"add_m_option" parameter.

Remove the parameter, and inside part_get_mbr_part_type(), remove the dead
branch.

Relatedly, update the comment on the
"print_partition_table_machine_readable" OCaml function, originally
from
commit 32e661f42169 ("daemon: Reimplement ?part_list? API in OCaml.",
2017-07-27). Because print_partition_table() now passes "-m" to
"parted"
unconditionally, and there are no use cases left that would *forbid*
"-m",
"print_partition_table_machine_readable" is almost equivalent to
print_partition_table() -- modulo the enforcement of the "BYT;"
header.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - new patch: unrelated refactoring I'd found possible while working the
      BZ

 daemon/parted.c  | 17 ++++++-----------
 daemon/parted.ml |  6 ++----
 2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/daemon/parted.c b/daemon/parted.c
index ef7b90056b41..3a723762c345 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -341,7 +341,7 @@ get_table_field (const char *line, int n)
 }
 
 static char *
-print_partition_table (const char *device, bool add_m_option)
+print_partition_table (const char *device)
 {
   char *out;
   CLEANUP_FREE char *err = NULL;
@@ -349,14 +349,9 @@ print_partition_table (const char *device, bool
add_m_option)
 
   udev_settle ();
 
-  if (add_m_option)
-    r = command (&out, &err, "parted", "-m",
"-s", "--", device,
-                 "unit", "b",
-                 "print", NULL);
-  else
-    r = command (&out, &err, "parted", "-s",
"--", device,
-                 "unit", "b",
-                 "print", NULL);
+  r = command (&out, &err, "parted", "-m",
"-s", "--", device,
+               "unit", "b",
+               "print", NULL);
 
   udev_settle ();
 
@@ -383,7 +378,7 @@ do_part_get_bootable (const char *device, int partnum)
     return -1;
   }
 
-  CLEANUP_FREE char *out = print_partition_table (device, true);
+  CLEANUP_FREE char *out = print_partition_table (device);
   if (!out)
     return -1;
 
@@ -555,7 +550,7 @@ do_part_get_name (const char *device, int partnum)
     return NULL;
 
   if (STREQ (parttype, "gpt")) {
-    CLEANUP_FREE char *out = print_partition_table (device, true);
+    CLEANUP_FREE char *out = print_partition_table (device);
     if (!out)
       return NULL;
 
diff --git a/daemon/parted.ml b/daemon/parted.ml
index 45c387987483..29b98c4806c1 100644
--- a/daemon/parted.ml
+++ b/daemon/parted.ml
@@ -57,10 +57,8 @@ let part_get_mbr_id device partnum    (* It's printed in
hex, possibly with a leading space. *)
   sscanf out " %x" identity
 
-(* This is not equivalent to print_partition_table in the C code, as
- * it only deals with the ?-m? option output, and it partially parses
- * that.  If we convert other functions that don't use the ?-m? version
- * we'll have to refactor this. XXX
+(* This is almost equivalent to print_partition_table in the C code. The
+ * difference is that here we enforce the "BYT;" header internally.
  *)
 let print_partition_table_machine_readable device    udev_settle ();
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2021-Nov-24 10:43 UTC

head link

[Libguestfs] [PATCH v2 3/5] daemon/parted: simplify print_partition_table() prototype

On 11/24/21 11:37, Laszlo Ersek wrote:> Since commit 994ca1f8ebcc ("daemon: Reimplement
'part_get_mbr_part_type'
> API in OCaml.", 2018-05-02), we've not had any calls to
> print_partition_table() that would pass a "true" argument for the
Typo: should be '"false" argument'.

Will fix upon merge or in v3.

Sorry!
Laszlo
> "add_m_option" parameter.
> 
> Remove the parameter, and inside part_get_mbr_part_type(), remove the dead
> branch.
> 
> Relatedly, update the comment on the
> "print_partition_table_machine_readable" OCaml function,
originally from
> commit 32e661f42169 ("daemon: Reimplement ?part_list? API in
OCaml.",
> 2017-07-27). Because print_partition_table() now passes "-m" to
"parted"
> unconditionally, and there are no use cases left that would *forbid*
"-m",
> "print_partition_table_machine_readable" is almost equivalent to
> print_partition_table() -- modulo the enforcement of the "BYT;"
header.
> 
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> 
> Notes:
>     v2:
>     
>     - new patch: unrelated refactoring I'd found possible while working
the
>       BZ
> 
>  daemon/parted.c  | 17 ++++++-----------
>  daemon/parted.ml |  6 ++----
>  2 files changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/daemon/parted.c b/daemon/parted.c
> index ef7b90056b41..3a723762c345 100644
> --- a/daemon/parted.c
> +++ b/daemon/parted.c
> @@ -341,7 +341,7 @@ get_table_field (const char *line, int n)
>  }
>  
>  static char *
> -print_partition_table (const char *device, bool add_m_option)
> +print_partition_table (const char *device)
>  {
>    char *out;
>    CLEANUP_FREE char *err = NULL;
> @@ -349,14 +349,9 @@ print_partition_table (const char *device, bool
add_m_option)
>  
>    udev_settle ();
>  
> -  if (add_m_option)
> -    r = command (&out, &err, "parted", "-m",
"-s", "--", device,
> -                 "unit", "b",
> -                 "print", NULL);
> -  else
> -    r = command (&out, &err, "parted", "-s",
"--", device,
> -                 "unit", "b",
> -                 "print", NULL);
> +  r = command (&out, &err, "parted", "-m",
"-s", "--", device,
> +               "unit", "b",
> +               "print", NULL);
>  
>    udev_settle ();
>  
> @@ -383,7 +378,7 @@ do_part_get_bootable (const char *device, int partnum)
>      return -1;
>    }
>  
> -  CLEANUP_FREE char *out = print_partition_table (device, true);
> +  CLEANUP_FREE char *out = print_partition_table (device);
>    if (!out)
>      return -1;
>  
> @@ -555,7 +550,7 @@ do_part_get_name (const char *device, int partnum)
>      return NULL;
>  
>    if (STREQ (parttype, "gpt")) {
> -    CLEANUP_FREE char *out = print_partition_table (device, true);
> +    CLEANUP_FREE char *out = print_partition_table (device);
>      if (!out)
>        return NULL;
>  
> diff --git a/daemon/parted.ml b/daemon/parted.ml
> index 45c387987483..29b98c4806c1 100644
> --- a/daemon/parted.ml
> +++ b/daemon/parted.ml
> @@ -57,10 +57,8 @@ let part_get_mbr_id device partnum >    (* It's
printed in hex, possibly with a leading space. *)
>    sscanf out " %x" identity
>  
> -(* This is not equivalent to print_partition_table in the C code, as
> - * it only deals with the ?-m? option output, and it partially parses
> - * that.  If we convert other functions that don't use the ?-m?
version
> - * we'll have to refactor this. XXX
> +(* This is almost equivalent to print_partition_table in the C code. The
> + * difference is that here we enforce the "BYT;" header
internally.
>   *)
>  let print_partition_table_machine_readable device >    udev_settle ();
>

Libguestfs - Nov 2021 - [PATCH v2 3/5] daemon/parted: simplify print_partition_table() prototype

[Libguestfs] [PATCH v2 3/5] daemon/parted: simplify print_partition_table() prototype

[Libguestfs] [PATCH v2 3/5] daemon/parted: simplify print_partition_table() prototype