Am 24.06.2014 18:04, schrieb 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 <p...@kamp.de>
>> ---
>> 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.
thanks for pointing that out. Actually I was grepping for strtoul in the
qemu source and found it in several places (also with no error checking).
I will change that.
>
> 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.
>
Right I will split that up.
Thanks for your comments,
Peter
