On Fri, Nov 12, 2010 at 09:42:45AM +0100, Clément Bœsch wrote:
> On Fri, Nov 12, 2010 at 08:14:32AM +0100, Reimar Döffinger wrote:
> > On Thu, Nov 11, 2010 at 10:57:00PM +0100, Clément Bœsch wrote:
> > > > Also please use "const char" instead of "char" (I think
> > > > you might have to change some more code to avoid new warnings,
> > > > but I think it should be possible without real issues).
> > >
> > > Sure, patch updated. It didn't generate a single warning.
> >
> > The warning would come in when you do it really correctly and
> > add the const also to the return value, which I'd consider
> > preferable.
>
> I see. Patch updated. I don't know how to test the libmenu modification,
> but it builds correctly with --enable-menu.
>
> --
> Clément B.
> Not sent from a jesusPhone.
> From 67b1c2cb08eedf4758616e7d6d550cfd5bc6916e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
> Date: Thu, 11 Nov 2010 17:30:36 +0100
> Subject: [PATCH] Export mp_basename in a function
>
> ---
> gui/interface.c | 2 --
> libmenu/menu_pt.c | 17 +++++++++++------
> mplayer.c | 11 ++++-------
> path.c | 13 +++++++++++++
> path.h | 1 +
> 5 files changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/gui/interface.c b/gui/interface.c
> index a03281e..a83aaa2 100644
> --- a/gui/interface.c
> +++ b/gui/interface.c
> @@ -1135,8 +1135,6 @@ void * gtkSet( int cmd,float fparam, void * vparam )
> return NULL;
> }
>
> -#define mp_basename(s) (strrchr(s,'/')==NULL?(char*)s:(strrchr(s,'/')+1))
> -
> #include "playtree.h"
>
> //This function adds/inserts one file into the gui playlist
> diff --git a/libmenu/menu_pt.c b/libmenu/menu_pt.c
> index 80d9cf7..984db1a 100644
> --- a/libmenu/menu_pt.c
> +++ b/libmenu/menu_pt.c
> @@ -24,6 +24,7 @@
> #include "config.h"
> #include "mp_msg.h"
> #include "help_mp.h"
> +#include "path.h"
>
> #include "libmpcodecs/img_format.h"
> #include "libmpcodecs/mp_image.h"
> @@ -38,8 +39,6 @@
> #include "input/input.h"
> #include "access_mpcontext.h"
>
> -#define mp_basename(s) (strrchr((s),'/')==NULL?(char*)(s):(strrchr((s),'/')+1))
> -
> struct list_entry_s {
> struct list_entry p;
> play_tree_t* pt;
> @@ -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.