[sheepdog] [PATCH v2 2/3] move parse_option_size to lib/option.c

[Hitoshi Mitake]

At Fri, 23 Aug 2013 20:40:53 +0800,
Liu Yuan wrote:
> 
> Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> ---
>  dog/vdi.c        |   42 ++++++------------------------------------
>  include/option.h |    1 +
>  lib/option.c     |   33 +++++++++++++++++++++++++++++++++
>  3 files changed, 40 insertions(+), 36 deletions(-)
> 
> diff --git a/dog/vdi.c b/dog/vdi.c
> index 3eb03dd..65c9bac 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..8f52371 100644
> --- a/lib/option.c
> +++ b/lib/option.c
> @@ -61,6 +61,39 @@ 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);
> +	switch (*postfix) {
> +	case 'T':
> +	case 't':
> +		sizef *= 1024;
> +	case 'G':
> +	case 'g':
> +		sizef *= 1024;
> +	case 'M':
> +	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;
> +}
> +

Current option_parse_size() interprets strings like "128GMB" as
correct things. This behavior wouldn't be able to catch configuration
mistakes.

How about this?

int option_parse_size(const char *value, uint64_t *ret)
{
	char *postfix;
	double sizef;

	sizef = strtod(value, &postfix);
	if (!(postfix[0] == '\0' || postfix[1] == 'b' ||
	      postfix[1] == 'B' || postfix[1] == '\0')) {
		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;
	}

	switch (postfix[0]) {
	case 'T':
	case 't':
		sizef *= 1024;
	case 'G':
	case 'g':
		sizef *= 1024;
	case 'M':
	case 'm':
		sizef *= 1024;
	case 'K':
	case 'k':
		sizef *= 1024;
	default:
		*ret = (uint64_t) sizef;
		break;
	}

	return 0;
}

Thanks,
Hitoshi