| Message ID | 20161202192223.15153-4-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02.12.2016 20:22, Eric Blake wrote: > Rather than store into a local variable, the copy to the struct > if the value is valid, then reporting errors otherwise, It is rather difficult to part this sentence starting from "the copy". > it is > simpler to just store into the struct and report errors if the > value is invalid. This however requires that the struct store > a 64-bit number, rather than a narrower type. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v3: new patch > --- > block/blkdebug.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) Anyway, thanks! If you can explain to me how to parse the commit message or make it easier to read: Reviewed-by: Max Reitz <mreitz@redhat.com>
On 12/06/2016 04:10 PM, Max Reitz wrote: > On 02.12.2016 20:22, Eric Blake wrote: >> Rather than store into a local variable, the copy to the struct >> if the value is valid, then reporting errors otherwise, > > It is rather difficult to part this sentence starting from "the copy". That's because I meant to type "then copy". (What a difference an 'n' makes - I hope it does't imply that the batteries i my wireless keyboard are o the declie agai, or a sig of ay other more serious problem) > >> it is >> simpler to just store into the struct and report errors if the >> value is invalid. This however requires that the struct store >> a 64-bit number, rather than a narrower type. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> --- >> v3: new patch >> --- >> block/blkdebug.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) > > Anyway, thanks! If you can explain to me how to parse the commit message > or make it easier to read: > > Reviewed-by: Max Reitz <mreitz@redhat.com> >
Am 02.12.2016 um 20:22 hat Eric Blake geschrieben: > Rather than store into a local variable, the copy to the struct > if the value is valid, then reporting errors otherwise, it is > simpler to just store into the struct and report errors if the > value is invalid. This however requires that the struct store > a 64-bit number, rather than a narrower type. > > Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/block/blkdebug.c b/block/blkdebug.c index aac8184..edc2b05 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -38,7 +38,7 @@ typedef struct BDRVBlkdebugState { int state; int new_state; - int align; + uint64_t align; /* For blkdebug_refresh_filename() */ char *config_file; @@ -353,7 +353,6 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, BDRVBlkdebugState *s = bs->opaque; QemuOpts *opts; Error *local_err = NULL; - uint64_t align; int ret; opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); @@ -389,11 +388,9 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, bs->file->bs->supported_zero_flags; /* Set request alignment */ - align = qemu_opt_get_size(opts, "align", 0); - if (align < INT_MAX && is_power_of_2(align)) { - s->align = align; - } else if (align) { - error_setg(errp, "Invalid alignment"); + s->align = qemu_opt_get_size(opts, "align", 0); + if (s->align && (s->align >= INT_MAX || !is_power_of_2(s->align))) { + error_setg(errp, "Invalid alignment, align must be integer power of 2"); ret = -EINVAL; goto fail_unref; }
Rather than store into a local variable, the copy to the struct if the value is valid, then reporting errors otherwise, it is simpler to just store into the struct and report errors if the value is invalid. This however requires that the struct store a 64-bit number, rather than a narrower type. Signed-off-by: Eric Blake <eblake@redhat.com> --- v3: new patch --- block/blkdebug.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)