[vlc-devel] [PATCH] upnp: add basic network interface discovery for iOS and tvOS

[Alexandre Janniaux]

Hi,

I have a few remarks below, but patch LGTM after that.

On Fri, Jul 03, 2020 at 12:59:50PM +0200, Felix Paul Kühne wrote:
> From: Felix Paul Kühne <felix at feepk.net>
>
> ---
>  modules/services_discovery/upnp.cpp | 33 +++++++++++++++++++++++++++--
>  1 file changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/modules/services_discovery/upnp.cpp b/modules/services_discovery/upnp.cpp
> index 13c1eb7971..c9cd08f053 100644
> --- a/modules/services_discovery/upnp.cpp
> +++ b/modules/services_discovery/upnp.cpp
> @@ -1621,9 +1621,38 @@ inline char *getPreferedAdapter()
>  }
>  #else
>
> -static char *getPreferedAdapter()
> +inline bool necessaryFlagsSetOnInterface(struct ifaddrs *anInterface)
>  {
> -    return NULL;
> +    unsigned int flags = anInterface->ifa_flags;
> +    if( (flags & IFF_UP) && (flags & IFF_RUNNING) && !(flags & IFF_LOOPBACK) && !(flags & IFF_POINTOPOINT) ) {

Do you also need IFF_MULTICAST?

> +        return true;
> +    }
> +    return false;
> +}
> +
> +inline char *getPreferedAdapter()
> +{
> +    struct ifaddrs *listOfInterfaces = NULL;
> +    struct ifaddrs *anInterface = NULL;

Could be left uninitialized in order to warn if it gets
used without calling the correct function/assignation in
future refactoring.

> +    int ret = getifaddrs(&listOfInterfaces);
> +    char *adapterName = NULL;
> +
> +    if (ret == 0) {

Could be changed into

    if (ret != 0)
        return NULL

Especially to avoid calling freeifaddr on NULL or
uninitialized memory, and remove an indentation level.

> +        anInterface = listOfInterfaces;
> +
> +        while (anInterface != NULL) {
> +            bool ret = necessaryFlagsSetOnInterface(anInterface);
> +            if (ret) {
> +                adapterName = strdup(anInterface->ifa_name);
> +                break;
> +            }
> +
> +            anInterface = anInterface->ifa_next;
> +        }
> +    }
> +    freeifaddrs(listOfInterfaces);
> +
> +    return adapterName;
>  }
>
>  #endif
> --
> 2.21.1 (Apple Git-122.3)
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel