Re: [PATCH] tests/socket-helpers: Only fail socket protocol check if it is really necessary

[Daniel P . Berrangé]

On Tue, Sep 08, 2020 at 12:54:35PM +0200, Thomas Huth wrote:
> The tests/test-char test is currently always failing on my system since
> socket_can_bind_connect("::1", PF_INET6) fails with EINVAL and thus
> socket_check_protocol_support() is returning -1 for an error. But IPv4
> is working fine. The logic in socket_check_protocol_support() seems to
> be wrong here, if either IPv6 or IPv4 is working, we should not return
> an error here. Thus rework the function to only return errors if both
> checks failed.

Can you tell me which exact syscall is giving EINVAL in this scenario ?

> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/socket-helpers.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/socket-helpers.c b/tests/socket-helpers.c
> index 19a51e887e..62a0e0f2d9 100644
> --- a/tests/socket-helpers.c
> +++ b/tests/socket-helpers.c
> @@ -136,22 +136,17 @@ static int socket_can_bind_connect(const char 
> *hostname, int family)
>  
>  int socket_check_protocol_support(bool *has_ipv4, bool *has_ipv6)
>  {
> -    *has_ipv4 = *has_ipv6 = false;
> +    int errv4, errv6;
>  
> -    if (socket_can_bind_connect("127.0.0.1", PF_INET) < 0) {
> -        if (errno != EADDRNOTAVAIL) {
> -            return -1;
> -        }
> -    } else {
> -        *has_ipv4 = true;
> -    }
> +    errv4 = socket_can_bind_connect("127.0.0.1", PF_INET);
> +    *has_ipv4 = (errv4 == 0);
>  
> -    if (socket_can_bind_connect("::1", PF_INET6) < 0) {
> -        if (errno != EADDRNOTAVAIL) {
> -            return -1;
> -        }
> -    } else {
> -        *has_ipv6 = true;
> +    errv6 = socket_can_bind_connect("::1", PF_INET6);
> +    *has_ipv6 = (errv6 == 0);
> +
> +    if (!*has_ipv4 && !*has_ipv6 &&
> +        (errv4 != EADDRNOTAVAIL || errv6 != EADDRNOTAVAIL)) {
> +        return -1;
>      }

The return value of socket_can_bind_connect is either 0 or -1,
but you're treating it an errno which isn't right.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|