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.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.
Thanks for your comments anyway, I'll update the commit message for the patch. > > 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". _______________________________________________ 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".