[vlc-devel] [PATCH] [WIP] Partial UPnP rewrite

[Hugo Beauzée-Luyssen]

On Tue, Mar 10, 2015, at 09:37 PM, Jean-Baptiste Kempf wrote:
> On 10 Mar, Hugo Beauzée-Luyssen wrote :
> >  modules/services_discovery/upnp.cpp | 1229 ++++++++++++-----------------------
> >  modules/services_discovery/upnp.hpp |  181 ++----
> >  2 files changed, 487 insertions(+), 923 deletions(-)
> 
> It's almost a rewrite, no?
> 
> > -static int Open( vlc_object_t* );
> > -static void Close( vlc_object_t* );
> > +namespace SD
> > +{
> > +    static int Open( vlc_object_t* );
> > +    static void Close( vlc_object_t* );
> > +}
> > +
> > +namespace Access
> > +{
> > +    static int Open( vlc_object_t* );
> > +    static void Close( vlc_object_t* );
> > +}
> 
> That is elegant :)
> 
> > +    p_sys->p_server_list = new(std::nothrow) SD::MediaServerList( p_sd );
> > +    if ( unlikely( p_sys->p_server_list == NULL ) )
> > +    {
> > +        return VLC_ENOMEM;
> > +    }
> 
> Can this happen?
> 

As much as malloc can return NULL.

> > +    /* Search for media servers */
> > +    int i_res = UpnpSearchAsync( p_sys->p_upnp->handle(), 5,
> > +            MEDIA_SERVER_DEVICE_TYPE, p_sys->p_server_list );
> 
> Search is in the Open?
> 

SearchAsync :)
As far as I can see, SD modules are always launching the discovery from
the Open callback, and results/events are processed from an asynchronous
callbacks/another thread

> > diff --git a/modules/services_discovery/upnp.hpp b/modules/services_discovery/upnp.hpp
> > index 23fe4db..971b709 100644
> > --- a/modules/services_discovery/upnp.hpp
> > +++ b/modules/services_discovery/upnp.hpp
> 
> Do we need a .hpp ?

I don't see any obvious reason to have it, beside readability. It's
doable to implement everything within the class declaration, but I don't
like it that much.

> 
> With my kindest regards,
> 

Regards,

-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr