[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/7] qapi: use qemu_strtoi64() in parse_str |
Date: |
Wed, 31 Oct 2018 17:47:13 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 |
On 31.10.18 15:40, Markus Armbruster wrote:
> David Hildenbrand <address@hidden> writes:
>
>> The qemu api claims to be easier to use, and the resulting code seems to
>> agree.
>
> Ah, an opportunity to nitpick spelling! "The QEMU API", and "qapi: Use
> qemu_strtoi64() ..."
Whatever floats your boat ;) Will change.
>
>> Signed-off-by: David Hildenbrand <address@hidden>
>> ---
>> qapi/string-input-visitor.c | 17 ++++++-----------
>> 1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
>> index b3fdd0827d..c1454f999f 100644
>> --- a/qapi/string-input-visitor.c
>> +++ b/qapi/string-input-visitor.c
>> @@ -20,6 +20,7 @@
>> #include "qemu/option.h"
>> #include "qemu/queue.h"
>> #include "qemu/range.h"
>> +#include "qemu/cutils.h"
>>
>>
>> struct StringInputVisitor
>> @@ -46,10 +47,10 @@ static void free_range(void *range, void *dummy)
>>
>> static int parse_str(StringInputVisitor *siv, const char *name, Error
>> **errp)
>> {
>> - char *str = (char *) siv->string;
>> - long long start, end;
>> + const char *str = (char *) siv->string;
>
> And an opportunity to nitpick whitespace! Drop the space between (char
> *) and siv->string while there.
Shouldn't checkpatch complain, too? Will fix.
>
>> + const char *endptr;
>> + int64_t start, end;
>> Range *cur;
>> - char *endptr;
>>
>> if (siv->ranges) {
>> return 0;
>> @@ -60,9 +61,7 @@ static int parse_str(StringInputVisitor *siv, const char
>> *name, Error **errp)
>> }
>>
>> do {
>> - errno = 0;
>> - start = strtoll(str, &endptr, 0);
>> - if (errno == 0 && endptr > str) {
>> + if (!qemu_strtoi64(str, &endptr, 0, &start)) {
>> if (*endptr == '\0') {
>> cur = g_malloc0(sizeof(*cur));
>> range_set_bounds(cur, start, start);
>> @@ -71,11 +70,7 @@ static int parse_str(StringInputVisitor *siv, const char
>> *name, Error **errp)
>> str = NULL;
>> } else if (*endptr == '-') {
>> str = endptr + 1;
>> - errno = 0;
>> - end = strtoll(str, &endptr, 0);
>> - if (errno == 0 && endptr > str && start <= end &&
>> - (start > INT64_MAX - 65536 ||
>> - end < start + 65536)) {
>> + if (!qemu_strtoi64(str, &endptr, 0, &end) && start < end) {
>
> You deleted (start > INT64_MAX - 65536 || end < start + 65536). Can you
> explain that to me? I'm feeling particularly dense today...
qemu_strtoi64 performs all different kinds of error handling completely
internally. This old code here was an attempt to filter out -EWHATEVER
from the response. No longer needed as errors and the actual value are
reported via different ways.
Thanks!
>
>> if (*endptr == '\0') {
>> cur = g_malloc0(sizeof(*cur));
>> range_set_bounds(cur, start, end);
--
Thanks,
David / dhildenb
[Qemu-devel] [PATCH v3 7/7] memory-device: rewrite address assignment using ranges, David Hildenbrand, 2018/10/23
[Qemu-devel] [PATCH v3 5/7] memory-device: use QEMU_IS_ALIGNED, David Hildenbrand, 2018/10/23
Re: [Qemu-devel] [PATCH v3 0/7] qapi/range/memory-device: fixes and cleanups, David Hildenbrand, 2018/10/31