Re: [Qemu-devel] [PATCH 1/5] Introduce strtobytes() library function to convert string to byte count.

[Andreas Färber]

Am 15.09.2010 um 14:23 schrieb address@hidden:
[...]
> +/*
> + * Convert string to bytes, allowing either K/k for KB, M/m for MB,
> + * G/b for GB or T/t for TB. Default without any postfix is MB.
      ^^^ typo
> + * End pointer will be returned in *end, if end is valid.
> + * Return 0 on error.
> + */

You seem to be refactoring existing code into this function, but the  
use of such suffixes usually brings up the question whether it's  
factor 1024 or 1000.
Here you're using 1024 apparently. If you don't want the user dealing  
with (imo ugly) Ki/Mi/Gi/Ti units this should at least be documented  
accordingly: G/g for GiB, etc. or G/g for GB = 1024 MB, etc.

Andreas

> +uint64_t strtobytes(const char *nptr, char **end)
> +{
> +    uint64_t value;
> +    char *endptr;
> +
> +    value = strtoll(nptr, &endptr, 0);
> +    switch (*endptr++) {
> +    case 'K':
> +    case 'k':
> +        value <<= 10;
> +        break;
> +    case 0:
> +    case 'M':
> +    case 'm':
> +        value <<= 20;
> +        break;
> +    case 'G':
> +    case 'g':
> +        value <<= 30;
> +        break;
> +    case 'T':
> +    case 't':
> +        value <<= 40;
> +        break;
> +    default:
> +        value = 0;
> +    }
> +
> +    if (end)
> +        *end = endptr;
> +
> +    return value;
> +}
> diff --git a/qemu-common.h b/qemu-common.h
> index dfd3dc0..01d7dcb 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -137,6 +137,7 @@ time_t mktimegm(struct tm *tm);
> int qemu_fls(int i);
> int qemu_fdatasync(int fd);
> int fcntl_setfl(int fd, int flag);
> +uint64_t strtobytes(const char *nptr, char **end);
>
> /* path.c */
> void init_paths(const char *prefix);
> diff --git a/vl.c b/vl.c
> index 3f45aa9..0cdd94a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -734,14 +734,10 @@ static void numa_add(const char *optarg)
>         if (get_param_value(option, 128, "mem", optarg) == 0) {
>             node_mem[nodenr] = 0;
>         } else {
> -            value = strtoull(option, &endptr, 0);
> -            switch (*endptr) {
> -            case 0: case 'M': case 'm':
> -                value <<= 20;
> -                break;
> -            case 'G': case 'g':
> -                value <<= 30;
> -                break;
> +            value = strtobytes(option, NULL);
> +            if (!value) {
> +                fprintf(stderr, "qemu: invalid numa mem size: %s 
> \n", optarg);
> +                exit(1);
>             }
>             node_mem[nodenr] = value;
>         }
> @@ -2166,17 +2162,9 @@ int main(int argc, char **argv, char **envp)
>                 break;
>             case QEMU_OPTION_m: {
>                 uint64_t value;
> -                char *ptr;
>
> -                value = strtoul(optarg, &ptr, 10);
> -                switch (*ptr) {
> -                case 0: case 'M': case 'm':
> -                    value <<= 20;
> -                    break;
> -                case 'G': case 'g':
> -                    value <<= 30;
> -                    break;
> -                default:
> +                value = strtobytes(optarg, NULL);
> +                if (!value) {
>                     fprintf(stderr, "qemu: invalid ram size: %s\n",  
> optarg);
>                     exit(1);
>                 }
> --
> 1.7.2.2