[vlc-devel] [PATCH 1/7] input: also copy input slaves in input_item_Copy

[Alexandre Janniaux]

Hi,

Ok for the two points then, they should probably be fixed
together later but it can be out of this patchset, as error
case is also not specified in the documention.

LGTM.

Regards,
--
Alexandre Janniaux
Videolabs

On Tue, Oct 22, 2019 at 09:45:11AM +0200, Pierre Lamot wrote:
> On 2019-10-22 00:51, Alexandre Janniaux wrote:
> > Hi,
> >
> > This is meaningful as the slaves are owned by the input
> > item, but maybe the documentation of the input_item_Copy
> > should be updated to signal this?
>
> the documentation is very broad "This function creates a new input_item_t as
> a copy of another",
> I kinda expect it to give me a 1:1 copy of the input_item (maybe epg and
> categories should be copied as well)
>
> though, I guess I can detail what is copied here.
>
> > The case of failure is a bit weird. If we cannot allocate
> > memory it would only remove the slave silently from the
> > copy. Maybe bailing out and return NULL would be a better
> > error management candidate?
>
> the vlc_meta_Merge and the input_item_CopyOptions called within the function
> may already fail silently.
>
> > Regards,
> > --
> > Alexandre Janniaux
> > Videolabs
> >
> > On Mon, Oct 21, 2019 at 06:08:53PM +0200, Pierre Lamot wrote:
> > > ---
> > >  src/input/item.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/src/input/item.c b/src/input/item.c
> > > index e8db39ca28..a01d4bb3e4 100644
> > > --- a/src/input/item.c
> > > +++ b/src/input/item.c
> > > @@ -1119,6 +1119,22 @@ input_item_t *input_item_Copy( input_item_t
> > > *p_input )
> > >          vlc_meta_Merge( meta, p_input->p_meta );
> > >      }
> > >      b_net = p_input->b_net;
> > > +
> > > +    if( likely(item != NULL) && p_input->i_slaves > 0 )
> > > +    {
> > > +        for( int i = 0; i < p_input->i_slaves; i++ )
> > > +        {
> > > +            input_item_slave_t* slave = input_item_slave_New(
> > > +                        p_input->pp_slaves[i]->psz_uri,
> > > +                        p_input->pp_slaves[i]->i_type,
> > > +                        p_input->pp_slaves[i]->i_priority);
> > > +            if( unlikely(slave != NULL) )
> > > +            {
> > > +                TAB_APPEND(item->i_slaves, item->pp_slaves, slave);
> > > +            }
> > > +        }
> > > +    }
> > > +
> > >      vlc_mutex_unlock( &p_input->lock );
> > >
> > >      if( likely(item != NULL) )
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > 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