[MPlayer-dev-eng] Export mp_basename in a function

[Clément Bœsch]

On Tue, Nov 16, 2010 at 10:55:00PM +0100, Reimar Döffinger wrote:
> On Tue, Nov 16, 2010 at 09:28:59PM +0100, Clément Bœsch wrote:
> > On Tue, Nov 16, 2010 at 09:09:57PM +0100, Reimar Döffinger wrote:
> > > > @@ -124,8 +123,14 @@ static int read_key(menu_t* menu,int c){
> > > >    return menu_list_jump_to_key(menu, c);
> > > >  }
> > > >  
> > > > +static void free_entry(list_entry_t* entry)
> > > > +{
> > > > +  free(entry->p.txt);
> > > > +  free(entry);
> > > > +}
> > > > +
> > > >  static void close_menu(menu_t* menu) {
> > > > -  menu_list_uninit(menu,NULL);
> > > > +  menu_list_uninit(menu, free_entry);
> > > >  }
> > > >  
> > > >  static int op(menu_t* menu, char* args) {
> > > > @@ -146,7 +151,7 @@ static int op(menu_t* menu, char* args) {
> > > >  
> > > >    if(playtree_iter->tree->parent != playtree_iter->root) {
> > > >      e = calloc(1,sizeof(list_entry_t));
> > > > -    e->p.txt = "..";
> > > > +    e->p.txt = strdup("..");
> > > >      e->pt = playtree_iter->tree->parent;
> > > >      menu_list_add_entry(menu,e);
> > > >    }
> > > > @@ -156,9 +161,9 @@ static int op(menu_t* menu, char* args) {
> > > >    for( ; i != NULL ; i = i->next ) {
> > > >      e = calloc(1,sizeof(list_entry_t));
> > > >      if(i->files)
> > > > -      e->p.txt = mp_basename(i->files[0]);
> > > > +      e->p.txt = strdup(mp_basename(i->files[0]));
> > > >      else
> > > > -      e->p.txt = "Group ...";
> > > > +      e->p.txt = strdup("Group ...");
> > > >      e->pt = i;
> > > >      menu_list_add_entry(menu,e);
> > > >    }
> > > 
> > > You shouldn't add strdup()/free(), this mp_basename
> > > change really isn't supposed to change the behaviour.
> > > p.txt should be const, and if that generates warnings
> > > those should probably be suppressed by an explicit
> > > cast + documentation why it is safe.
> > 
> > Ok then, I'm going to commit it with the cast, but i'd like to make a
> > second commit to use strdup()/free() for a few reasons:
> > 
> >  - other modules in libmenu allocate and free the .txt fields entries just
> >    like that
> 
> That is (I hope) because it's actually necessary for those.
> 

So it should be justified for menu_pt.c now too :)

> >  - I don't get the purpose of making it const and then immediately make
> >    exceptions, I found the cast solution a bit ugly.
> 
> Granted, it doesn't make sense to use const here in general.
> Using a union might be an option, but it is not very nice either.
> 

OK with the attached patch then? I followed the same schema as
menu_dvbin.c.

> >  - it seems there is a leak of the list entry anyway, so a free_entry
> >    function is needed
> 
> No, uninit calls free() if no free function was given.

Oh ok I missed that.

> Feel free to propose improvements, but I'll probably want to see
> valgrind logs that show it actually fixing something - because
> the alternative is that I'd have to get familiar with that code
> and that's not really something I want to spend my time on.
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng

-- 
Clément B.
Not sent from a jesusPhone.