Re: [Qemu-block] [PATCH v2 1/1] block/sheepdog: fix argument passed to qemu_strtoul()

[Paolo Bonzini]

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
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