[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/7] qemu-img: add more conv= conversions to
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/7] qemu-img: add more conv= conversions to dd |
Date: |
Tue, 13 Sep 2016 17:06:30 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
On Fri, Aug 26, 2016 at 11:16:39AM +0200, Reda Sallahi wrote:
> @@ -3968,7 +3963,11 @@ static int img_dd_flag(const char *arg, struct DdIo
> *io,
> int j;
> for (j = 0; flags[j].name != NULL; j++) {
> if (!strcmp(tok, flags[j].name)) {
> - io->flags |= flags[j].value;
> + if (dd) {
> + dd->conv |= flags[j].value;
> + } else {
> + io->flags |= flags[j].value;
> + }
> break;
> }
> }
Does this function need to know about DdIo and DdInfo? How about:
static int img_dd_flag(const char *arg, unsigned int *flag_bits,
const struct DdSymbols *flags, const char *err_str)
Then function can be called with &dd->flags or &io->flags.
If you do this then please squash the change into earlier patches in
this series using git-rebase(1). That way the function is created with
its final prototype in the earliest patch that needs it and there is no
code churn.
> @@ -4219,6 +4260,7 @@ static int img_dd(int argc, char **argv)
> ret = -1;
> goto out;
> }
> + local_err = NULL;
Why set local_err to NULL here and not where it's declared? If it could
possibly be non-NULL then error_free() must be used to avoid a memory
leak.
> + if (dd.conv & C_SPARSE) {
> + if (buffer_is_zero(in.buf, bsz)) {
> + sparse_count++;
> + continue;
> + }
> + if (sparse_count > 0) {
> + out_pos += sparse_count * in.bsz;
> + sparse_count = 0;
> + }
> + }
Is sparse_count needed at all?
if ((dd.conv & C_SPARSE) && buffer_is_zero(in.buf, bsz)) {
out_pos += in.bsz;
continue;
}
signature.asc
Description: PGP signature
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 3/7] qemu-img: add more conv= conversions to dd,
Stefan Hajnoczi <=