01.12.2018 1:03, Eric Blake wrote: > Our open-coding of strtol handling forgot to handle overflow > conditions. What's more, since we insiste on a user-supplied > partition to be non-zero, we can use 0 rather than -1 for our > initial value to distinguish when a partition is not being > served, for slightly more optimal code. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > qemu-nbd.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 55e29bd9a7e..866e64779f1 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -546,7 +546,7 @@ int main(int argc, char **argv) > int opt_ind = 0; > char *end; > int flags = BDRV_O_RDWR; > - int partition = -1; > + int partition = 0; > int ret = 0; > bool seen_cache = false; > bool seen_discard = false; > @@ -685,13 +685,9 @@ int main(int argc, char **argv) > flags &= ~BDRV_O_RDWR; > break; > case 'P': > - partition = strtol(optarg, &end, 0); > - if (*end) { > - error_report("Invalid partition `%s'", optarg); > - exit(EXIT_FAILURE); > - } > - if (partition < 1 || partition > 8) { > - error_report("Invalid partition %d", partition); > + if (qemu_strtoi(optarg, NULL, 0, &partition) < 0 ||
I decided to look into qemu_strtoi, hmm. is it possible, that "char *ep" remains uninitialized, and than we access it in check_strtox_error? I don's see in strtol spec a guarantee of setting endptr on failure path. > + partition < 1 || partition > 8) { don't you like brace on separate line after multi-line conditions? > + error_report("Invalid partition %s", optarg); > exit(EXIT_FAILURE); > } > break; > @@ -1004,7 +1000,7 @@ int main(int argc, char **argv) > } > fd_size -= dev_offset; > > - if (partition != -1) { > + if (partition) { > ret = find_partition(blk, partition, &dev_offset, &fd_size); > if (ret < 0) { > error_report("Could not find partition %d: %s", partition, > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> -- Best regards, Vladimir