Re: [Qemu-devel] [PATCH 3/7] qemu-img: add more conv= conversions to dd

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2489 bytes --]

On Mon, Aug 22, 2016 at 09:55:13AM +0200, Reda Sallahi wrote:
>  static int img_dd_conv(const char *arg,
>                         struct DdIo *in, struct DdIo *out,
>                         struct DdInfo *dd)
>  {
> -    if (!strcmp(arg, "notrunc")) {
> -        dd->conv |= C_NOTRUNC;
> -        return 0;
> -    } else {
> -        error_report("invalid conversion: '%s'", arg);
> -        return 1;
> +    const char *tok;
> +    char *str, *tmp;
> +    int ret = 0;
> +    const struct DdSymbols conv[] = {
> +        { "notrunc", C_NOTRUNC },
> +        { "sync", C_SYNC },
> +        { "noerror", C_NOERROR },
> +        { "fdatasync", C_FDATASYNC },
> +        { "fsync", C_FSYNC },
> +        { "excl", C_EXCL },
> +        { "nocreat", C_NOCREAT },
> +        { "sparse", C_SPARSE },
> +        { NULL, 0 }
> +    };
> +
> +    tmp = str = g_strdup(arg);
> +
> +    while (tmp != NULL && !ret) {
> +        tok = qemu_strsep(&tmp, ",");
> +        int j;
> +        for (j = 0; conv[j].name != NULL; j++) {
> +            if (!strcmp(tok, conv[j].name)) {
> +                if ((dd->conv | conv[j].value) & C_EXCL &&
> +                    (dd->conv | conv[j].value) & C_NOCREAT) {
> +                    error_report("cannot combine excl and nocreat");
> +                    ret = 1;
> +                    break;
> +                }
> +                dd->conv |= conv[j].value;
> +                break;
> +            }
> +        }
> +        if (conv[j].name == NULL) {
> +            error_report("invalid conversion: '%s'", tok);
> +            ret = 1;
> +        }
>      }
> +
> +    g_free(str);
> +    return ret;
>  }

This function is very similar to img_dd_iflag/oflag.  The code
duplication can be avoided if you perform the (dd->conv | conv[j].value)
& C_EXCL && (dd->conv | conv[j].value) & C_NOCREAT later.

> @@ -4325,20 +4388,43 @@ static int img_dd(int argc, char **argv)
>  
>      for (out_pos = out.offset * obsz; in_pos < size; block_count++) {
>          int in_ret, out_ret;
> +        bsz = in.bsz;
>  
>          if (in_pos + in.bsz > size) {
> -            in_ret = blk_pread(blk1, in_pos, in.buf, size - in_pos);
> -        } else {
> -            in_ret = blk_pread(blk1, in_pos, in.buf, in.bsz);
> +            bsz = size - in_pos;
> +        }
> +
> +        if (dd.conv & C_SYNC) {
> +            memset(in.buf, 0, in.bsz);
>          }

Why is memset necessary?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]