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

[Alexandre Janniaux]

(you probably want to mention that it's for 3.0 though).

What is expected for 4.0?

Regards,
--
Alexandre Janniaux
Videolabs

On Mon, Jul 06, 2020 at 11:37:46AM +0200, Alexandre Janniaux wrote:
> 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel