[Libguestfs] [PATCH 1/2] parted: introduce enum for whether parted has option -m

[Pino Toscano]

On Tuesday 24 March 2015 07:20:16 Chen Hanxiao wrote:
> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> ---
>  daemon/parted.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/daemon/parted.c b/daemon/parted.c
> index a7bcb99..64a7d3c 100644
> --- a/daemon/parted.c
> +++ b/daemon/parted.c
> @@ -33,6 +33,12 @@ GUESTFSD_EXT_CMD(str_parted, parted);
>  GUESTFSD_EXT_CMD(str_sfdisk, sfdisk);
>  GUESTFSD_EXT_CMD(str_sgdisk, sgdisk);
>  
> +enum {
> +  PARTED_INVALID = -1,
> +  /* parted do not support -m option */
> +  PARTED_OPT_NO_M,
> +  PARTED_OPT_HAS_M};

(I didn't have even the time to reply to the question about this enum)

PARTED_INVALID does not make much sense, especially that I was
referring just to the parameter for print_partition_table, so for it
only two values (eg PARTED_OUTPUT_MACHINE and PARTED_OUTPUT_NORMAL)
are enough.


> +
>  /* Notes:
>   *
>   * Parted 1.9 sends error messages to stdout, hence use of the
> @@ -320,7 +326,7 @@ get_table_field (const char *line, int n)
>  static int
>  test_parted_m_opt (void)
>  {
> -  static int result = -1;
> +  static int result = PARTED_INVALID;

Commenting here, but it applies to the rest of the changes: if you want
to apply an enum for this case, then do it consistently and for all the
cases. Using an enum value and storing it to an int variable defeats
the point of using an enum at all, as you will not catch non-enum
values or not check to be handling all values where needed.

-- 
Pino Toscano