[MPlayer-dev-eng] [PATCH] Subtitles directories

[Clément Bœsch]

On Sun, Nov 21, 2010 at 12:05:58PM +0100, Clément Bœsch wrote:
> On Sat, Nov 20, 2010 at 09:49:44PM +0100, Reimar Döffinger wrote:
> > On Sat, Nov 20, 2010 at 09:21:34PM +0100, Clément Bœsch wrote:
> > > + * \brief Allocates a new buffer containing the directory name
> > > + * \param path Original path. Must be a valid string.
> > > + *
> > > + * The path returned always contains a trailing slash '/'.
> > > + * On systems supporting DOS paths, '\' is also considered as a directory
> > > + * separator in addition to the '/'.
> > > + */
> > > +char *mp_dirname(const char *path)
> > > +{
> > > +    const char *base = mp_basename(path);
> > > +    size_t len = base - path;
> > > +    char *dirname;
> > > +
> > > +    if (len == 0)
> > > +        return strdup("./");
> > > +    dirname = malloc(len + 1);
> > > +    if (!dirname)
> > > +        return NULL;
> > > +    strncpy(dirname, path, len);
> > > +    dirname[len] = '\0';
> > 
> > Not sure if it is an improvement, but you could do both in
> > one by using
> > av_strlcpy(dirname, path, len + 1);
> 
> After looking at the av_strlcpy implementation, it seems a trivial copy is
> made, so I found the strncpy more appropriated since it can benefits libc
> optimizations.
> 
> > Either way, that part is ok.
> > Except for one thing I noticed while looking through the next
> > patch: For DOS paths you must also consider ':' a path separator
> > in mp_basename, so that c:test.avi is treated correctly.
> > 
> 
> OK, then I'll commit this, and make a second patch to fix that.
> 

Applied.

What about this patch for the ':' separator?

According to these tests, it works:

  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"));

  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"));

But since c:test.avi must be supported, I wonder if c:foo:test.avi has to.
The current patch consider ':' as a normal path separator, just like
subreader.c does, but maybe a smarter thing must be done?

-- 
Clément B.
Not sent from a jesusPhone.
-------------- next part --------------
Index: path.c
===================================================================
--- path.c	(revision 32641)
+++ path.c	(working copy)
@@ -202,6 +202,9 @@
     s = strrchr(path, '\\');
     if (s)
         path = s + 1;
+    s = strrchr(path, ':');
+    if (s)
+        path = s + 1;
 #endif
     s = strrchr(path, '/');
     return s ? s + 1 : path;