diff mbox

[FFmpeg-devel,v3,3/3] avutil/avstring: replace with system interface if dirname and basename functions are detected

Message ID 20191023155517.335-3-lance.lmwang@gmail.com
State New
Headers show

Commit Message

Lance Wang Oct. 23, 2019, 3:55 p.m. UTC 
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 configure            |  4 ++++
 libavutil/avstring.c | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Paul B Mahol Oct. 23, 2019, 4:07 p.m. UTC | #1 
Why?

On 10/23/19, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  configure            |  4 ++++
>  libavutil/avstring.c | 13 ++++++++++++-
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 8413826..5296a4c 100755
> --- a/configure
> +++ b/configure
> @@ -2188,6 +2188,8 @@ SYSTEM_FUNCS="
>      clock_gettime
>      closesocket
>      CommandLineToArgvW
> +    dirname
> +    basename
>      fcntl
>      getaddrinfo
>      gethrtime
> @@ -5980,6 +5982,8 @@ check_func  access
>  check_func_headers stdlib.h arc4random
>  check_lib   clock_gettime time.h clock_gettime || check_lib clock_gettime
> time.h clock_gettime -lrt
>  check_func  fcntl
> +check_func_headers libgen.h dirname
> +check_func_headers libgen.h basename
>  check_func  fork
>  check_func  gethrtime
>  check_func  getopt
> diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> index 76a13ba..66654fb 100644
> --- a/libavutil/avstring.c
> +++ b/libavutil/avstring.c
> @@ -19,12 +19,15 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
>   */
>
> +#include "config.h"
>  #include <stdarg.h>
>  #include <stdint.h>
>  #include <stdio.h>
>  #include <string.h>
> +#if defined(HAVE_DIRNAME) || defined(HAVE_BASENAME)
> +#include <libgen.h>
> +#endif
>
> -#include "config.h"
>  #include "common.h"
>  #include "mem.h"
>  #include "avassert.h"
> @@ -257,6 +260,9 @@ char *av_strireplace(const char *str, const char *from,
> const char *to)
>
>  const char *av_basename(const char *path)
>  {
> +#if HAVE_BASENAME
> +    return  basename((char*)path);
> +#else
>      char *p;
>
>      if (!path || *path == '\0')
> @@ -274,10 +280,14 @@ const char *av_basename(const char *path)
>          return path;
>
>      return p + 1;
> +#endif
>  }
>
>  const char *av_dirname(char *path)
>  {
> +#if HAVE_DIRNAME
> +    return dirname(path);
> +#else
>      char *p = path ? strrchr(path, '/') : NULL;
>
>  #if HAVE_DOS_PATHS
> @@ -295,6 +305,7 @@ const char *av_dirname(char *path)
>      *p = '\0';
>
>      return path;
> +#endif
>  }
>
>  char *av_append_path_component(const char *path, const char *component)
> --
> 2.6.4
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang Oct. 25, 2019, 1:11 a.m. UTC | #2 
On Wed, Oct 23, 2019 at 06:07:24PM +0200, Paul B Mahol wrote:
> Why?
> 
Sorry,  I  haven't get your question, you mean why to use system 
standard c lib API?

> On 10/23/19, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  configure            |  4 ++++
> >  libavutil/avstring.c | 13 ++++++++++++-
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure b/configure
> > index 8413826..5296a4c 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2188,6 +2188,8 @@ SYSTEM_FUNCS="
> >      clock_gettime
> >      closesocket
> >      CommandLineToArgvW
> > +    dirname
> > +    basename
> >      fcntl
> >      getaddrinfo
> >      gethrtime
> > @@ -5980,6 +5982,8 @@ check_func  access
> >  check_func_headers stdlib.h arc4random
> >  check_lib   clock_gettime time.h clock_gettime || check_lib clock_gettime
> > time.h clock_gettime -lrt
> >  check_func  fcntl
> > +check_func_headers libgen.h dirname
> > +check_func_headers libgen.h basename
> >  check_func  fork
> >  check_func  gethrtime
> >  check_func  getopt
> > diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> > index 76a13ba..66654fb 100644
> > --- a/libavutil/avstring.c
> > +++ b/libavutil/avstring.c
> > @@ -19,12 +19,15 @@
> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> > USA
> >   */
> >
> > +#include "config.h"
> >  #include <stdarg.h>
> >  #include <stdint.h>
> >  #include <stdio.h>
> >  #include <string.h>
> > +#if defined(HAVE_DIRNAME) || defined(HAVE_BASENAME)
> > +#include <libgen.h>
> > +#endif
> >
> > -#include "config.h"
> >  #include "common.h"
> >  #include "mem.h"
> >  #include "avassert.h"
> > @@ -257,6 +260,9 @@ char *av_strireplace(const char *str, const char *from,
> > const char *to)
> >
> >  const char *av_basename(const char *path)
> >  {
> > +#if HAVE_BASENAME
> > +    return  basename((char*)path);
> > +#else
> >      char *p;
> >
> >      if (!path || *path == '\0')
> > @@ -274,10 +280,14 @@ const char *av_basename(const char *path)
> >          return path;
> >
> >      return p + 1;
> > +#endif
> >  }
> >
> >  const char *av_dirname(char *path)
> >  {
> > +#if HAVE_DIRNAME
> > +    return dirname(path);
> > +#else
> >      char *p = path ? strrchr(path, '/') : NULL;
> >
> >  #if HAVE_DOS_PATHS
> > @@ -295,6 +305,7 @@ const char *av_dirname(char *path)
> >      *p = '\0';
> >
> >      return path;
> > +#endif
> >  }
> >
> >  char *av_append_path_component(const char *path, const char *component)
> > --
> > 2.6.4
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol Oct. 25, 2019, 7:51 a.m. UTC | #3 
On 10/25/19, Limin Wang <lance.lmwang@gmail.com> wrote:
> On Wed, Oct 23, 2019 at 06:07:24PM +0200, Paul B Mahol wrote:
>> Why?
>>
> Sorry,  I  haven't get your question, you mean why to use system
> standard c lib API?

What are gains?

>
>> On 10/23/19, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote:
>> > From: Limin Wang <lance.lmwang@gmail.com>
>> >
>> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>> > ---
>> >  configure            |  4 ++++
>> >  libavutil/avstring.c | 13 ++++++++++++-
>> >  2 files changed, 16 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/configure b/configure
>> > index 8413826..5296a4c 100755
>> > --- a/configure
>> > +++ b/configure
>> > @@ -2188,6 +2188,8 @@ SYSTEM_FUNCS="
>> >      clock_gettime
>> >      closesocket
>> >      CommandLineToArgvW
>> > +    dirname
>> > +    basename
>> >      fcntl
>> >      getaddrinfo
>> >      gethrtime
>> > @@ -5980,6 +5982,8 @@ check_func  access
>> >  check_func_headers stdlib.h arc4random
>> >  check_lib   clock_gettime time.h clock_gettime || check_lib
>> > clock_gettime
>> > time.h clock_gettime -lrt
>> >  check_func  fcntl
>> > +check_func_headers libgen.h dirname
>> > +check_func_headers libgen.h basename
>> >  check_func  fork
>> >  check_func  gethrtime
>> >  check_func  getopt
>> > diff --git a/libavutil/avstring.c b/libavutil/avstring.c
>> > index 76a13ba..66654fb 100644
>> > --- a/libavutil/avstring.c
>> > +++ b/libavutil/avstring.c
>> > @@ -19,12 +19,15 @@
>> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> > 02110-1301
>> > USA
>> >   */
>> >
>> > +#include "config.h"
>> >  #include <stdarg.h>
>> >  #include <stdint.h>
>> >  #include <stdio.h>
>> >  #include <string.h>
>> > +#if defined(HAVE_DIRNAME) || defined(HAVE_BASENAME)
>> > +#include <libgen.h>
>> > +#endif
>> >
>> > -#include "config.h"
>> >  #include "common.h"
>> >  #include "mem.h"
>> >  #include "avassert.h"
>> > @@ -257,6 +260,9 @@ char *av_strireplace(const char *str, const char
>> > *from,
>> > const char *to)
>> >
>> >  const char *av_basename(const char *path)
>> >  {
>> > +#if HAVE_BASENAME
>> > +    return  basename((char*)path);
>> > +#else
>> >      char *p;
>> >
>> >      if (!path || *path == '\0')
>> > @@ -274,10 +280,14 @@ const char *av_basename(const char *path)
>> >          return path;
>> >
>> >      return p + 1;
>> > +#endif
>> >  }
>> >
>> >  const char *av_dirname(char *path)
>> >  {
>> > +#if HAVE_DIRNAME
>> > +    return dirname(path);
>> > +#else
>> >      char *p = path ? strrchr(path, '/') : NULL;
>> >
>> >  #if HAVE_DOS_PATHS
>> > @@ -295,6 +305,7 @@ const char *av_dirname(char *path)
>> >      *p = '\0';
>> >
>> >      return path;
>> > +#endif
>> >  }
>> >
>> >  char *av_append_path_component(const char *path, const char *component)
>> > --
>> > 2.6.4
>> >
>> > _______________________________________________
>> > ffmpeg-devel mailing list
>> > ffmpeg-devel@ffmpeg.org
>> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >
>> > To unsubscribe, visit link above, or email
>> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Lance Wang Oct. 25, 2019, 2:42 p.m. UTC | #4 
On Fri, Oct 25, 2019 at 09:51:33AM +0200, Paul B Mahol wrote:
> On 10/25/19, Limin Wang <lance.lmwang@gmail.com> wrote:
> > On Wed, Oct 23, 2019 at 06:07:24PM +0200, Paul B Mahol wrote:
> >> Why?
> >>
> > Sorry,  I  haven't get your question, you mean why to use system
> > standard c lib API?
> 
> What are gains?

Our implementation haven't handle some corner case, one example is
the check for trailing slash characters.The system library interface
is widely used by most applications, and bugs should be less.

> 
> >
> >> On 10/23/19, lance.lmwang@gmail.com <lance.lmwang@gmail.com> wrote:
> >> > From: Limin Wang <lance.lmwang@gmail.com>
> >> >
> >> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >> > ---
> >> >  configure            |  4 ++++
> >> >  libavutil/avstring.c | 13 ++++++++++++-
> >> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/configure b/configure
> >> > index 8413826..5296a4c 100755
> >> > --- a/configure
> >> > +++ b/configure
> >> > @@ -2188,6 +2188,8 @@ SYSTEM_FUNCS="
> >> >      clock_gettime
> >> >      closesocket
> >> >      CommandLineToArgvW
> >> > +    dirname
> >> > +    basename
> >> >      fcntl
> >> >      getaddrinfo
> >> >      gethrtime
> >> > @@ -5980,6 +5982,8 @@ check_func  access
> >> >  check_func_headers stdlib.h arc4random
> >> >  check_lib   clock_gettime time.h clock_gettime || check_lib
> >> > clock_gettime
> >> > time.h clock_gettime -lrt
> >> >  check_func  fcntl
> >> > +check_func_headers libgen.h dirname
> >> > +check_func_headers libgen.h basename
> >> >  check_func  fork
> >> >  check_func  gethrtime
> >> >  check_func  getopt
> >> > diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> >> > index 76a13ba..66654fb 100644
> >> > --- a/libavutil/avstring.c
> >> > +++ b/libavutil/avstring.c
> >> > @@ -19,12 +19,15 @@
> >> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> >> > 02110-1301
> >> > USA
> >> >   */
> >> >
> >> > +#include "config.h"
> >> >  #include <stdarg.h>
> >> >  #include <stdint.h>
> >> >  #include <stdio.h>
> >> >  #include <string.h>
> >> > +#if defined(HAVE_DIRNAME) || defined(HAVE_BASENAME)
> >> > +#include <libgen.h>
> >> > +#endif
> >> >
> >> > -#include "config.h"
> >> >  #include "common.h"
> >> >  #include "mem.h"
> >> >  #include "avassert.h"
> >> > @@ -257,6 +260,9 @@ char *av_strireplace(const char *str, const char
> >> > *from,
> >> > const char *to)
> >> >
> >> >  const char *av_basename(const char *path)
> >> >  {
> >> > +#if HAVE_BASENAME
> >> > +    return  basename((char*)path);
> >> > +#else
> >> >      char *p;
> >> >
> >> >      if (!path || *path == '\0')
> >> > @@ -274,10 +280,14 @@ const char *av_basename(const char *path)
> >> >          return path;
> >> >
> >> >      return p + 1;
> >> > +#endif
> >> >  }
> >> >
> >> >  const char *av_dirname(char *path)
> >> >  {
> >> > +#if HAVE_DIRNAME
> >> > +    return dirname(path);
> >> > +#else
> >> >      char *p = path ? strrchr(path, '/') : NULL;
> >> >
> >> >  #if HAVE_DOS_PATHS
> >> > @@ -295,6 +305,7 @@ const char *av_dirname(char *path)
> >> >      *p = '\0';
> >> >
> >> >      return path;
> >> > +#endif
> >> >  }
> >> >
> >> >  char *av_append_path_component(const char *path, const char *component)
> >> > --
> >> > 2.6.4
> >> >
> >> > _______________________________________________
> >> > ffmpeg-devel mailing list
> >> > ffmpeg-devel@ffmpeg.org
> >> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> >
> >> > To unsubscribe, visit link above, or email
> >> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox

Patch

diff --git a/configure b/configure
index 8413826..5296a4c 100755
--- a/configure
+++ b/configure
@@ -2188,6 +2188,8 @@  SYSTEM_FUNCS="
     clock_gettime
     closesocket
     CommandLineToArgvW
+    dirname
+    basename
     fcntl
     getaddrinfo
     gethrtime
@@ -5980,6 +5982,8 @@  check_func  access
 check_func_headers stdlib.h arc4random
 check_lib   clock_gettime time.h clock_gettime || check_lib clock_gettime time.h clock_gettime -lrt
 check_func  fcntl
+check_func_headers libgen.h dirname
+check_func_headers libgen.h basename
 check_func  fork
 check_func  gethrtime
 check_func  getopt
diff --git a/libavutil/avstring.c b/libavutil/avstring.c
index 76a13ba..66654fb 100644
--- a/libavutil/avstring.c
+++ b/libavutil/avstring.c
@@ -19,12 +19,15 @@ 
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include "config.h"
 #include <stdarg.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
+#if defined(HAVE_DIRNAME) || defined(HAVE_BASENAME)
+#include <libgen.h>
+#endif
 
-#include "config.h"
 #include "common.h"
 #include "mem.h"
 #include "avassert.h"
@@ -257,6 +260,9 @@  char *av_strireplace(const char *str, const char *from, const char *to)
 
 const char *av_basename(const char *path)
 {
+#if HAVE_BASENAME
+    return  basename((char*)path);
+#else
     char *p;
 
     if (!path || *path == '\0')
@@ -274,10 +280,14 @@  const char *av_basename(const char *path)
         return path;
 
     return p + 1;
+#endif
 }
 
 const char *av_dirname(char *path)
 {
+#if HAVE_DIRNAME
+    return dirname(path);
+#else
     char *p = path ? strrchr(path, '/') : NULL;
 
 #if HAVE_DOS_PATHS
@@ -295,6 +305,7 @@  const char *av_dirname(char *path)
     *p = '\0';
 
     return path;
+#endif
 }
 
 char *av_append_path_component(const char *path, const char *component)