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

[Marton Balint]

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.

Regards,
Marton