On Wed, Mar 02, 2016 at 05:12:57PM +0100, Paolo Bonzini wrote: > > > On 02/03/2016 17:09, Jeff Cody wrote: > > + ret = qemu_strtoul(snapshot_id, NULL, 10, &snap_id); > > + if (ret) { > > + error_setg_errno(errp, -ret, "Invalid snapshot ID: %s", > > + snapshot_id ? snapshot_id : "<null>"); > > + return ret; > > + } > > + > > + if (snap_id > UINT32_MAX) { > > + error_setg_errno(errp, EINVAL, "Snapshot ID numeric value %" PRId64 > > + " exceeds Sheepdog maximum of %" PRId32, snap_id, > > + UINT32_MAX); > > + return -EINVAL; > > } > > I think including the errno produces a worse error message ("Invalid > snapshot ID: foo: Invalid argument" or something like that), and also
Hey, if the user enters invalid input, they deserve a cryptic error message! Oh, alright - sent a v3 :-) > the error should be the same for an id of 10^10 (within uint64_t bounds) > or 10^30 (outside the bounds). So you could just use > > if (ret < 0 || snap_id > UINT32_MAX) { > error_setg(errp, "Invalid snapshot ID: %s", > snapshot_id ? snapshot_id : "<null>"); > return -EINVAL; > } > > Thanks, > > Paolo