[FFmpeg-devel] [PATCH v2 3/7] avformat/hlsenc: Check some unchecked allocations

[Limin Wang]

On Thu, Apr 09, 2020 at 11:48:10AM +0200, Andreas Rheinhardt wrote:
> Steven Liu:
> > 
> > 
> >> 2020年2月28日 下午5:53，Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道：
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> >> ---
> >> libavformat/hlsenc.c | 13 ++++++++++++-
> >> 1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> >> index a281c379f0..18f40ff3ed 100644
> >> --- a/libavformat/hlsenc.c
> >> +++ b/libavformat/hlsenc.c
> >> @@ -1610,6 +1610,8 @@ static int hls_start(AVFormatContext *s, VariantStream *vs)
> >>             if (c->use_localtime_mkdir) {
> >>                 const char *dir;
> >>                 char *fn_copy = av_strdup(oc->url);
> >> +                if (!fn_copy)
> >> +                    return AVERROR(ENOMEM);
> >>                 dir = av_dirname(fn_copy);
> >>                 if (ff_mkdir_p(dir) == -1 && errno != EEXIST) {
> >>                     av_log(oc, AV_LOG_ERROR, "Could not create directory %s with use_localtime_mkdir\n", dir);
> >> @@ -1770,6 +1772,8 @@ static int validate_name(int nb_vs, const char *fn)
> >>     }
> >>
> >>     fn_dup = av_strdup(fn);
> >> +    if (!fn_dup)
> >> +        return AVERROR(ENOMEM);
> >>     filename = av_basename(fn);
> >>     subdir_name = av_dirname(fn_dup);
> >>
> >> @@ -2139,6 +2143,8 @@ static int update_master_pl_info(AVFormatContext *s)
> >>     int ret = 0;
> >>
> >>     fn1 = av_strdup(s->url);
> >> +    if (!fn1)
> >> +        return AVERROR(ENOMEM);
> > It’s unnecessary here,
> > I have checked all strdup return checker in hlsenc some month ago, and double check the workflow in update_master_pl_info,
> > It's the safe whether you check the strdup or not.
> > Reference commit id: 61aa77272a25d83e5ce5c63d93c64bb9a3e15557
> 
> If these strdups fail, the relevant dirnames will be wrong. While you
> don't get segfaults, you will not create the files at the right
> destinations. We should rather error out instead.

Sorry, it seems that I remove these checking when av_dirname claims to support
NULL for path in the API comments like glib dirname function. So I think it's
duplicate check if the function claims to support NULL. If have different
opinions, please add the checking anyway. I don't know why dirname support NULL? 

> 
> - Andreas
> _______________________________________________
> 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".

-- 
Thanks,
Limin Wang