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.lmw...@gmail.com: >>From: Limin Wang <lance.lmw...@gmail.com> >> >>Signed-off-by: Limin Wang <lance.lmw...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
