> -----Original Message----- > From: [email protected] [mailto:[email protected]] On > Behalf Of Richard W.M. Jones > Sent: Tuesday, March 24, 2015 9:31 PM > To: Pino Toscano > Cc: [email protected] > Subject: Re: [Libguestfs] [PATCH 1/2] parted: introduce enum for whether > parted > has option -m > > On Tue, Mar 24, 2015 at 01:15:21PM +0100, Pino Toscano wrote: > > On Tuesday 24 March 2015 07:20:16 Chen Hanxiao wrote: > > > Signed-off-by: Chen Hanxiao <[email protected]> > > > --- > > > 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. > > Agreed. > > However I'm just going to make this fix and push it. >
Hi Rich, Pino Thanks for your help. Regards, - Chen _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
