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

[Tomas Härdin]

ons 2019-09-18 klockan 06:41 +0800 skrev Limin Wang:
> 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.

Sure. Better would of course be to not be able to parse invalid input
at all

> 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.

*cries in langsec*

/Tomas