At Mon, 26 Aug 2013 14:09:17 +0800,
Liu Yuan wrote:
> 
> On Mon, Aug 26, 2013 at 03:04:53PM +0900, MORITA Kazutaka wrote:
> > At Mon, 26 Aug 2013 13:54:53 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Mon, Aug 26, 2013 at 02:51:05PM +0900, MORITA Kazutaka wrote:
> > > > At Mon, 26 Aug 2013 13:42:58 +0800,
> > > > Liu Yuan wrote:
> > > > > 
> > > > > Signed-off-by: Liu Yuan <[email protected]>
> > > > > ---
> > > > >  dog/vdi.c        |   42 ++++++------------------------------------
> > > > >  include/option.h |    1 +
> > > > >  lib/option.c     |   37 +++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 44 insertions(+), 36 deletions(-)
> > > > > 
> > > > > diff --git a/dog/vdi.c b/dog/vdi.c
> > > > > index 0ce3785..30f686b 100644
> > > > > --- a/dog/vdi.c
> > > > > +++ b/dog/vdi.c
> > > > > @@ -55,36 +55,6 @@ struct get_vdi_info {
> > > > >       uint8_t nr_copies;
> > > > >  };
> > > > >  
> > > > > -static int parse_option_size(const char *value, uint64_t *ret)
> > > > > -{
> > > > > -     char *postfix;
> > > > > -     double sizef;
> > > > > -
> > > > > -     sizef = strtod(value, &postfix);
> > > > > -     switch (*postfix) {
> > > > > -     case 'T':
> > > > > -             sizef *= 1024;
> > > > > -     case 'G':
> > > > > -             sizef *= 1024;
> > > > > -     case 'M':
> > > > > -             sizef *= 1024;
> > > > > -     case 'K':
> > > > > -     case 'k':
> > > > > -             sizef *= 1024;
> > > > > -     case 'b':
> > > > > -     case '\0':
> > > > > -             *ret = (uint64_t) sizef;
> > > > > -             break;
> > > > > -     default:
> > > > > -             sd_err("Invalid size '%s'", value);
> > > > > -             sd_err("You may use k, M, G or T suffixes for "
> > > > > -                    "kilobytes, megabytes, gigabytes and 
> > > > > terabytes.");
> > > > > -             return -1;
> > > > > -     }
> > > > > -
> > > > > -     return 0;
> > > > > -}
> > > > > -
> > > > >  static void vdi_show_progress(uint64_t done, uint64_t total)
> > > > >  {
> > > > >       return show_progress(done, total, false);
> > > > > @@ -502,7 +472,7 @@ static int vdi_create(int argc, char **argv)
> > > > >               sd_err("Please specify the VDI size");
> > > > >               return EXIT_USAGE;
> > > > >       }
> > > > > -     ret = parse_option_size(argv[optind], &size);
> > > > > +     ret = option_parse_size(argv[optind], &size);
> > > > >       if (ret < 0)
> > > > >               return EXIT_USAGE;
> > > > >       if (size > SD_MAX_VDI_SIZE) {
> > > > > @@ -699,7 +669,7 @@ static int vdi_resize(int argc, char **argv)
> > > > >               sd_err("Please specify the new size for the VDI");
> > > > >               return EXIT_USAGE;
> > > > >       }
> > > > > -     ret = parse_option_size(argv[optind], &new_size);
> > > > > +     ret = option_parse_size(argv[optind], &new_size);
> > > > >       if (ret < 0)
> > > > >               return EXIT_USAGE;
> > > > >       if (new_size > SD_MAX_VDI_SIZE) {
> > > > > @@ -1160,11 +1130,11 @@ static int vdi_read(int argc, char **argv)
> > > > >       char *buf = NULL;
> > > > >  
> > > > >       if (argv[optind]) {
> > > > > -             ret = parse_option_size(argv[optind++], &offset);
> > > > > +             ret = option_parse_size(argv[optind++], &offset);
> > > > >               if (ret < 0)
> > > > >                       return EXIT_USAGE;
> > > > >               if (argv[optind]) {
> > > > > -                     ret = parse_option_size(argv[optind++], &total);
> > > > > +                     ret = option_parse_size(argv[optind++], &total);
> > > > >                       if (ret < 0)
> > > > >                               return EXIT_USAGE;
> > > > >               }
> > > > > @@ -1234,11 +1204,11 @@ static int vdi_write(int argc, char **argv)
> > > > >       bool create;
> > > > >  
> > > > >       if (argv[optind]) {
> > > > > -             ret = parse_option_size(argv[optind++], &offset);
> > > > > +             ret = option_parse_size(argv[optind++], &offset);
> > > > >               if (ret < 0)
> > > > >                       return EXIT_USAGE;
> > > > >               if (argv[optind]) {
> > > > > -                     ret = parse_option_size(argv[optind++], &total);
> > > > > +                     ret = option_parse_size(argv[optind++], &total);
> > > > >                       if (ret < 0)
> > > > >                               return EXIT_USAGE;
> > > > >               }
> > > > > diff --git a/include/option.h b/include/option.h
> > > > > index 3a1eb20..ba62496 100644
> > > > > --- a/include/option.h
> > > > > +++ b/include/option.h
> > > > > @@ -31,6 +31,7 @@ char *build_short_options(const struct sd_option 
> > > > > *opts);
> > > > >  struct option *build_long_options(const struct sd_option *opts);
> > > > >  const char *option_get_help(const struct sd_option *, int);
> > > > >  int option_parse(char *arg, const char *delim, struct option_parser 
> > > > > *parsers);
> > > > > +int option_parse_size(const char *value, uint64_t *ret);
> > > > >  
> > > > >  #define sd_for_each_option(opt, opts)                \
> > > > >       for (opt = (opts); opt->name; opt++)
> > > > > diff --git a/lib/option.c b/lib/option.c
> > > > > index 39a4c52..d12c205 100644
> > > > > --- a/lib/option.c
> > > > > +++ b/lib/option.c
> > > > > @@ -61,6 +61,43 @@ const char *option_get_help(const struct sd_option 
> > > > > *sd_opts, int ch)
> > > > >       return NULL;
> > > > >  }
> > > > >  
> > > > > +int option_parse_size(const char *value, uint64_t *ret)
> > > > > +{
> > > > > +     char *postfix;
> > > > > +     double sizef;
> > > > > +
> > > > > +     sizef = strtod(value, &postfix);
> > > > > +     if (postfix[0] != '\0' && postfix[1] != '\0')
> > > > > +             goto err;
> > > > 
> > > > This makes the following command an error.
> > > > 
> > > >   $ dog vdi create test 4MB
> > > >   Invalid size '4MB'
> > > >   You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes 
> > > > and terabytes.
> > > 
> > > Why we need set 4MB instead of 4M? Allow only one subfix isn't simpler?
> > 
> > We have supported the notation like '4MB' so far.  If you want to
> > change the behavior, please note it to the commit log.
> > 
> > I'm not sure we really need to give up the backward compatibility,
> > though.
> > 
> 
> Technically, it is not we support 4MB, it is because we don't check anything
> after 'M' letter. With old code, I can even 
> 
> $ dog vdi create test 4Megabytes
> 
> I don't think it means we support 'Megabytes, Kilobytes, Gigabytes ...' 
> notation

Okay, it makes sense to me.

Thanks,

Kazutaka
-- 
sheepdog mailing list
[email protected]
http://lists.wpkg.org/mailman/listinfo/sheepdog

Reply via email to