Am 02.03.2016 um 17:12 hat Paolo Bonzini geschrieben:
>
>
> 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)
Yes. Rule of thumb: If you call error_setg_errno() with a constant
errno, there's something wrong.
Kevin
> , and also
> 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
