Re: [Qemu-devel] [PATCHv2] block/nfs: add knob to set readahead

[Eric Blake]

On 06/24/2014 02:52 AM, Peter Lieven wrote:
> upcoming libnfs will feature internal readahead support.
> Add a knob to pass the optional readahead value as a URL
> parameter.
> 
> This patch fixes also the incorrect usage of strncmp and
> atoi.
> 
> Signed-off-by: Peter Lieven <address@hidden>
> ---
> v1->v2: use strtol instead of atoi [Eric]
> 
>  block/nfs.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index ec43201..9783483 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -309,12 +309,16 @@ static int64_t nfs_client_open(NFSClient *client, const 
> char *filename,
>                         qp->p[i].name);
>              goto fail;
>          }
> -        if (!strncmp(qp->p[i].name, "uid", 3)) {
> -            nfs_set_uid(client->context, atoi(qp->p[i].value));
> -        } else if (!strncmp(qp->p[i].name, "gid", 3)) {
> -            nfs_set_gid(client->context, atoi(qp->p[i].value));
> -        } else if (!strncmp(qp->p[i].name, "tcp-syncnt", 10)) {
> -            nfs_set_tcp_syncnt(client->context, atoi(qp->p[i].value));
> +        if (!strcmp(qp->p[i].name, "uid")) {
> +            nfs_set_uid(client->context, strtol(qp->p[i].value, NULL, 0));

If you're going to use strtol, use it correctly.  You have to pre-set
errno, then check that something got parsed, that errno was not changed,
and that no unexpected suffix remains.  Better is using a wrapper that
already makes the parsing sane; such as parse_uint() from util/cutils.c.
 That is, swapping atoi() with a raw strtol() with no additional error
checking is not fixing any of the bugs inherent in the fact that atoi()
cannot detect overflow in user input.

At this point, I'd rather see this split into two patches - one that
fixes the atoi() usage, and another that adds readahead support - rather
than trying to cram two things in one commit.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature