Re: [Qemu-devel] [PATCH 2/7] qemu-img: add iflag and oflag options to dd

[Stefan Hajnoczi <stefanha@gmail.com>]

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

On Mon, Aug 22, 2016 at 09:55:12AM +0200, Reda Sallahi wrote:
> +static int img_dd_oflag(const char *arg,
> +                        struct DdIo *in, struct DdIo *out,
> +                        struct DdInfo *dd)
> +{
> +    const char *tok;
> +    char *str, *tmp;
> +    int ret = 0;
> +    const struct DdSymbols flags[] = {
> +        { "direct", C_DIRECT },
> +        { "dsync", C_DSYNC },
> +        { "sync", C_IOFLAG_SYNC },
> +        { "seek_bytes", C_SEEK_BYTES },
> +        { NULL, 0 }
> +    };
> +
> +    tmp = str = g_strdup(arg);
> +
> +    while (tmp != NULL && !ret) {
> +        tok = qemu_strsep(&tmp, ",");
> +        int j;
> +        for (j = 0; flags[j].name != NULL; j++) {
> +            if (!strcmp(tok, flags[j].name)) {
> +                out->flags |= flags[j].value;
> +                break;
> +            }
> +        }
> +        if (flags[j].name == NULL) {
> +            error_report("invalid output flag: '%s'", tok);
> +            ret = 1;
> +        }
> +    }
> +
> +    g_free(str);
> +    return ret;
> +}

img_dd_iflag()/img_dd_oflag() are duplicated code.  I suggest a single
helper function that parses a DdSymbols array into a DdIo->flags field.
Then you can pass in either the iflag or the oflag DdSymbols arrays.

static int img_dd_oflag(const char *arg,
                        struct DdIo *in, struct DdIo *out,
                        struct DdInfo *dd)
{
    return img_dd_xflag(arg, (DdSymbols[]){
    	{ "direct", C_DIRECT },
	{ "dsync", C_DSYNC },
	...
    }, in);
}

static int img_dd_oflag(const char *arg,
                        struct DdIo *in, struct DdIo *out,
                        struct DdInfo *dd)
{
    return img_dd_xflag(arg, (DdSymbols[]){
    	{ "direct", C_DIRECT },
	{ "dsync", C_DSYNC },
	...
    }, out);
}

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