[FFmpeg-devel] [PATCH v1 1/4] avutil/avstring: support input path is a null pointer or empty string

Limin Wang lance.lmwang at gmail.com
Thu Sep 19 04:08:22 EEST 2019


On Wed, Sep 18, 2019 at 08:25:40PM +0200, Marton Balint wrote:
> 
> 
> On Wed, 18 Sep 2019, Limin Wang wrote:
> 
> >On Tue, Sep 17, 2019 at 06:22:39PM +0200, Marton Balint wrote:
> >>
> >>
> >>On Mon, 16 Sep 2019, Tomas Härdin wrote:
> >>
> >>>mån 2019-09-16 klockan 09:03 +0800 skrev lance.lmwang at gmail.com:
> >>>>From: Limin Wang <lance.lmwang at gmail.com>
> >>>>
> >>>>Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> >>>>---
> >>>> libavutil/avstring.c | 12 ++++++++----
> >>>> libavutil/avstring.h | 13 +++++++++----
> >>>> 2 files changed, 17 insertions(+), 8 deletions(-)
> >>>>
> >>>>diff --git a/libavutil/avstring.c b/libavutil/avstring.c
> >>>>index 4c068f5bc5..9fddd0c77b 100644
> >>>>--- a/libavutil/avstring.c
> >>>>+++ b/libavutil/avstring.c
> >>>>@@ -257,8 +257,12 @@ char *av_strireplace(const char *str, const char
> >>>>*from, const char *to)
> >>>>
> >>>> const char *av_basename(const char *path)
> >>>> {
> >>>>-    char *p = strrchr(path, '/');
> >>>>+    char *p = NULL;
> >>>>+
> >>>>+    if (!path || *path == '\0')
> >>>>+        return ".";
> >>>
> >>>I will note here that this kind of thing would go great with a contract
> >>>on the function prototype, so that callers could formally verify that
> >>>they can indeed remove the NULL checks, and that the result of
> >>>av_basename() is always a valid string..
> >>
> >>This is basename, not dirname. We should not return an arbitrary
> >>(valid) value for invalid inputs.
> >
> >basename and dirname is supported by Linux and OSX system by <libgen.h>,
> >I consider to make the interface is consistent with the standard api first,
> >then it's time to change to invoke the system api if it's support. I
> >have read the implementaion in linux, it's more robust and tested. for
> >example, the current code haven't process multiple `/' characters.
> >
> >You can get more descrioption about the system api by below command:
> >man 3 basename
> 
> OK thanks for explaining, so "." is not completely arbitrary, it
> mimics the standard C API. Please add this as the reason of your
> change into the commit message. I am still not sure if it is good
> practice though, but I am fine with it if nobody else sees this
> problematic.

Thanks for your comments anyway, I'll update the commit message for the patch.

> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list