Re: [Qemu-devel] [PATCH v2 2/6] tests/vhost-user-bridge: Fix misuse of isdigit()

[Markus Armbruster]

Thomas Huth  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 
>>> ---
>>>  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, >sin_addr)) {
>>>  fprintf(stderr, "inet_aton() failed.\n");
>>>  exit(1);
>> 
>> Reviewed-by: Thomas Huth 
>
> 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.

Re: [Qemu-devel] [PATCH v2 2/6] tests/vhost-user-bridge: Fix misuse of isdigit()

[Thomas Huth]

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 
>> ---
>>  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, >sin_addr)) {
>>  fprintf(stderr, "inet_aton() failed.\n");
>>  exit(1);
> 
> Reviewed-by: Thomas Huth 

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

Re: [Qemu-devel] [PATCH v2 2/6] tests/vhost-user-bridge: Fix misuse of isdigit()

[Thomas Huth]

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 
> ---
>  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, >sin_addr)) {
>  fprintf(stderr, "inet_aton() failed.\n");
>  exit(1);

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 2/6] tests/vhost-user-bridge: Fix misuse of isdigit()

[Philippe Mathieu-Daudé]

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 
> ---
>  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, >sin_addr)) {
>  fprintf(stderr, "inet_aton() failed.\n");
>  exit(1);
> 

Reviewed-by: Philippe Mathieu-Daudé