| Message ID | 20190514180311.16028-3-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix misuse of ctype.h functions | expand |
On 5/14/19 8:03 PM, Markus Armbruster wrote: > vubr_set_host() passes char values to isdigit(). Undefined behavior "happens"? > when the value is negative. > > Fix by using qemu_isdigit() instead. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/vhost-user-bridge.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > index 0033b61f2e..d70b107ebc 100644 > --- a/tests/vhost-user-bridge.c > +++ b/tests/vhost-user-bridge.c > @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev) > static void > vubr_set_host(struct sockaddr_in *saddr, const char *host) > { > - if (isdigit(host[0])) { > + if (qemu_isdigit(host[0])) { > if (!inet_aton(host, &saddr->sin_addr)) { > fprintf(stderr, "inet_aton() failed.\n"); > exit(1); > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On 14/05/2019 20.03, Markus Armbruster wrote: > vubr_set_host() passes char values to isdigit(). Undefined behavior > when the value is negative. > > Fix by using qemu_isdigit() instead. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/vhost-user-bridge.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c > index 0033b61f2e..d70b107ebc 100644 > --- a/tests/vhost-user-bridge.c > +++ b/tests/vhost-user-bridge.c > @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev) > static void > vubr_set_host(struct sockaddr_in *saddr, const char *host) > { > - if (isdigit(host[0])) { > + if (qemu_isdigit(host[0])) { > if (!inet_aton(host, &saddr->sin_addr)) { > fprintf(stderr, "inet_aton() failed.\n"); > exit(1); Reviewed-by: Thomas Huth <thuth@redhat.com>
On 14/05/2019 20.41, Thomas Huth wrote: > On 14/05/2019 20.03, Markus Armbruster wrote: >> vubr_set_host() passes char values to isdigit(). Undefined behavior >> when the value is negative. >> >> Fix by using qemu_isdigit() instead. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/vhost-user-bridge.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c >> index 0033b61f2e..d70b107ebc 100644 >> --- a/tests/vhost-user-bridge.c >> +++ b/tests/vhost-user-bridge.c >> @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev) >> static void >> vubr_set_host(struct sockaddr_in *saddr, const char *host) >> { >> - if (isdigit(host[0])) { >> + if (qemu_isdigit(host[0])) { >> if (!inet_aton(host, &saddr->sin_addr)) { >> fprintf(stderr, "inet_aton() failed.\n"); >> exit(1); > > Reviewed-by: Thomas Huth <thuth@redhat.com> By the way, how do you compile / run this test? The original commit message say one should compile it with "make tests/vhost-user-bridge" but that does not work for me: $ make tests/vhost-user-bridge cc tests/vhost-user-bridge.c -o tests/vhost-user-bridge tests/vhost-user-bridge.c:32:24: fatal error: qemu/osdep.h: No such file or directory Thomas
Thomas Huth <thuth@redhat.com> writes: > On 14/05/2019 20.41, Thomas Huth wrote: >> On 14/05/2019 20.03, Markus Armbruster wrote: >>> vubr_set_host() passes char values to isdigit(). Undefined behavior >>> when the value is negative. >>> >>> Fix by using qemu_isdigit() instead. >>> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> tests/vhost-user-bridge.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c >>> index 0033b61f2e..d70b107ebc 100644 >>> --- a/tests/vhost-user-bridge.c >>> +++ b/tests/vhost-user-bridge.c Squashing in @@ -30,6 +30,7 @@ #define _FILE_OFFSET_BITS 64 #include "qemu/osdep.h" +#include "qemu-common.h" #include "qemu/atomic.h" #include "qemu/iov.h" #include "standard-headers/linux/virtio_net.h" >>> @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev) >>> static void >>> vubr_set_host(struct sockaddr_in *saddr, const char *host) >>> { >>> - if (isdigit(host[0])) { >>> + if (qemu_isdigit(host[0])) { >>> if (!inet_aton(host, &saddr->sin_addr)) { >>> fprintf(stderr, "inet_aton() failed.\n"); >>> exit(1); >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> > > By the way, how do you compile / run this test? The original commit > message say one should compile it with "make tests/vhost-user-bridge" > but that does not work for me: > > $ make tests/vhost-user-bridge > cc tests/vhost-user-bridge.c -o tests/vhost-user-bridge > tests/vhost-user-bridge.c:32:24: fatal error: qemu/osdep.h: No such file > or directory With that fixup, it compiles for me. Thanks for your question! I blindly assumed "make check" actually compiled this.
diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c index 0033b61f2e..d70b107ebc 100644 --- a/tests/vhost-user-bridge.c +++ b/tests/vhost-user-bridge.c @@ -645,7 +645,7 @@ vubr_host_notifier_setup(VubrDev *dev) static void vubr_set_host(struct sockaddr_in *saddr, const char *host) { - if (isdigit(host[0])) { + if (qemu_isdigit(host[0])) { if (!inet_aton(host, &saddr->sin_addr)) { fprintf(stderr, "inet_aton() failed.\n"); exit(1);
vubr_set_host() passes char values to isdigit(). Undefined behavior when the value is negative. Fix by using qemu_isdigit() instead. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/vhost-user-bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)