[Qemu-devel] [PATCH] qapi: fix input visitor bugs

[Qemu-devel] [PATCH] qapi: fix input visitor bugs

[Michael S. Tsirkin]

Remove dead code.  Reset errno to 0 before each strtoull call, as the
man page requires.

Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 qapi/string-input-visitor.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 72722e6..d8a8db0 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -48,11 +48,10 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
         return;
     }
 
-    errno = 0;
     do {
+        errno = 0;
         start = strtoll(str, &endptr, 0);
-        if (errno == 0 && endptr > str && INT64_MIN <= start &&
-            start <= INT64_MAX) {
+        if (errno == 0 && endptr > str) {
             if (*endptr == '\0') {
                 cur = g_malloc0(sizeof(*cur));
                 cur->begin = start;
@@ -63,9 +62,9 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
                 str = NULL;
             } else if (*endptr == '-') {
                 str = endptr + 1;
+                errno = 0;
                 end = strtoll(str, &endptr, 0);
-                if (errno == 0 && endptr > str &&
-                    INT64_MIN <= end && end <= INT64_MAX && start <= end &&
+                if (errno == 0 && endptr > str && start <= end &&
                     (start > INT64_MAX - 65536 ||
                      end < start + 65536)) {
                     if (*endptr == '\0') {
-- 
MST

Re: [Qemu-devel] [PATCH] qapi: fix input visitor bugs

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1720 bytes --]

On 06/18/2014 09:36 AM, Michael S. Tsirkin wrote:
> Remove dead code.  Reset errno to 0 before each strtoull call, as the
> man page requires.
> 
> Reported-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  qapi/string-input-visitor.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 72722e6..d8a8db0 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -48,11 +48,10 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
>          return;
>      }
>  
> -    errno = 0;
>      do {
> +        errno = 0;
>          start = strtoll(str, &endptr, 0);
> -        if (errno == 0 && endptr > str && INT64_MIN <= start &&
> -            start <= INT64_MAX) {
> +        if (errno == 0 && endptr > str) {

Based on this conditional...

>              if (*endptr == '\0') {
>                  cur = g_malloc0(sizeof(*cur));
>                  cur->begin = start;
> @@ -63,9 +62,9 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
>                  str = NULL;
>              } else if (*endptr == '-') {
>                  str = endptr + 1;
> +                errno = 0;

...this assignment to errno is dead code (we already know it is 0).  But
I'd rather leave it in for robustness sake (any change to intermediate
code may change the situation where we are no longer assured of the
current value of errno at this point).

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] qapi: fix input visitor bugs

[Michael S. Tsirkin]

On Wed, Jun 18, 2014 at 09:41:56AM -0600, Eric Blake wrote:
> On 06/18/2014 09:36 AM, Michael S. Tsirkin wrote:
> > Remove dead code.  Reset errno to 0 before each strtoull call, as the
> > man page requires.
> > 
> > Reported-by: Eric Blake <eblake@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  qapi/string-input-visitor.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > index 72722e6..d8a8db0 100644
> > --- a/qapi/string-input-visitor.c
> > +++ b/qapi/string-input-visitor.c
> > @@ -48,11 +48,10 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> >          return;
> >      }
> >  
> > -    errno = 0;
> >      do {
> > +        errno = 0;
> >          start = strtoll(str, &endptr, 0);
> > -        if (errno == 0 && endptr > str && INT64_MIN <= start &&
> > -            start <= INT64_MAX) {
> > +        if (errno == 0 && endptr > str) {
> 
> Based on this conditional...
> 
> >              if (*endptr == '\0') {
> >                  cur = g_malloc0(sizeof(*cur));
> >                  cur->begin = start;
> > @@ -63,9 +62,9 @@ static void parse_str(StringInputVisitor *siv, Error **errp)
> >                  str = NULL;
> >              } else if (*endptr == '-') {
> >                  str = endptr + 1;
> > +                errno = 0;
> 
> ...this assignment to errno is dead code (we already know it is 0).  But
> I'd rather leave it in for robustness sake (any change to intermediate
> code may change the situation where we are no longer assured of the
> current value of errno at this point).

Exactly, that's why I put it here.

> Reviewed-by: Eric Blake <eblake@redhat.com>


Thanks!

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