[MPlayer-cvslog] r32630 - in trunk: gui/interface.c libmenu/menu_pt.c mplayer.c path.c path.h

[Clément Bœsch]

On Wed, Nov 17, 2010 at 07:09:03PM +0300, Yuriy Kaminskiy wrote:
> Clément Bœsch wrote:
> > On Wed, Nov 17, 2010 at 06:33:22PM +0300, Yuriy Kaminskiy wrote:
> >> cboesch wrote:
> >>> Author: cboesch
> >>> Date: Tue Nov 16 22:06:52 2010
> >>> New Revision: 32630
> >>>
> >>> Log:
> >>> Export mp_basename in a function instead of duplicate macros in various places
> >>>
> >>> Modified:
> >>>    trunk/gui/interface.c
> >>>    trunk/libmenu/menu_pt.c
> >>>    trunk/mplayer.c
> >>>    trunk/path.c
> >>>    trunk/path.h
> >>>
> >> [...]
> >>> Modified: trunk/path.c
> >>> ==============================================================================
> >>> --- trunk/path.c	Tue Nov 16 20:58:54 2010	(r32629)
> >>> +++ trunk/path.c	Tue Nov 16 22:06:52 2010	(r32630)
> >>> @@ -193,3 +193,16 @@ void set_codec_path(const char *path)
> >>>      strcpy(codec_path, path);
> >>>      needs_free = 1;
> >>>  }
> >>> +
> >>> +const char *mp_basename(const char *path)
> >>> +{
> >>> +    char *s;
> >>> +
> >>> +#if HAVE_DOS_PATHS
> >>> +    s = strrchr(path, '\\');
> >>> +    if (s)
> >>> +        return s + 1;
> >>> +#endif
> >>> +    s = strrchr(path, '/');
> >>> +    return s ? s + 1 : s;
> >> 1) Return NULL and sigsegv on filenames without /.
> >> s/: s;/: path;/
> > 
> > Huh, strange I didn't noticed it, and it doesn't crash here. Anyway the
> 
> It crash in callers. strdup(NULL) -> crash (besides, it is just wrong;
> mp_basename("foo") should be "foo", not NULL, "",...).
> 

Yes I understand why it should crash, I just don't understand why I don't
get them. Anyway, I'll commit those fixes in a few minutes.

> >> 2) Not that I care about DoSish OS, but it does not work nice with both \ and /
> >> in paths, like c:\foo/bar.avi
> > What about c:/foo\bar.avi?
> 
> With \ after / - no problem with both your code and my modification. Problem was
> only with / after \ [note that unlike first patch, this is not regression,
> original macros had same problem IIRC]
> 

OK, looks good to me. A series of tests confirmed it:

  printf("%s\n", mp_basename("bar"));
  printf("%s\n", mp_basename("c:\\foo\\bar"));
  printf("%s\n", mp_basename("c:/foo/bar"));
  printf("%s\n", mp_basename("c:\\foo/bar"));
  printf("%s\n", mp_basename("c:/foo\\bar"));

(-> "bar" everytime)

Thanks, and sorry.

> >> @@ -201,7 +201,7 @@ const char *mp_basename(const char *path
> >>  #if HAVE_DOS_PATHS
> >>      s = strrchr(path, '\\');
> >>      if (s)
> >> -        return s + 1;
> >> +        path = s + 1;
> >>  #endif
> >>      s = strrchr(path, '/');
> >>      return s ? s + 1 : path;
> > 
> >> _______________________________________________
> >> MPlayer-cvslog mailing list
> >> MPlayer-cvslog at mplayerhq.hu
> >> https://lists.mplayerhq.hu/mailman/listinfo/mplayer-cvslog
> > 
> > 
> 
> _______________________________________________
> MPlayer-cvslog mailing list
> MPlayer-cvslog at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/mplayer-cvslog

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