| Message ID | 20191110040733.11755-2-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 99f505d2df521d83d53f7f9f9b359280f5af168b |
| Headers | show |
Andreas Rheinhardt: > In case an AVBPrint was not complete, icecast_open() would free some > buffers that have not been allocated yet instead of freeing the data of > the AVBPrint (if they have been allocated). Because this error does not > trigger a jump to the general cleanup section any more, one can moreover > remove a (now unnecessary) initialization of a pointer. > > Furthermore, finalizing an AVBPrint can fail (namely when the string > inside the AVBPrint has not been allocated yet) and so this needs to be > checked. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/icecast.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libavformat/icecast.c b/libavformat/icecast.c > index d2198b78ec..052cd37f3e 100644 > --- a/libavformat/icecast.c > +++ b/libavformat/icecast.c > @@ -89,7 +89,7 @@ static int icecast_open(URLContext *h, const char *uri, int flags) > > // URI part variables > char h_url[1024], host[1024], auth[1024], path[1024]; > - char *headers = NULL, *user = NULL; > + char *headers, *user = NULL; > int port, ret; > AVBPrint bp; > > @@ -105,10 +105,11 @@ static int icecast_open(URLContext *h, const char *uri, int flags) > cat_header(&bp, "Ice-Genre", s->genre); > cat_header(&bp, "Ice-Public", s->public ? "1" : "0"); > if (!av_bprint_is_complete(&bp)) { > - ret = AVERROR(ENOMEM); > - goto cleanup; > + av_bprint_finalize(&bp, NULL); > + return AVERROR(ENOMEM); > } > - av_bprint_finalize(&bp, &headers); > + if ((ret = av_bprint_finalize(&bp, &headers)) < 0) > + return ret; > > // Set options > av_dict_set(&opt_dict, "method", s->legacy_icecast ? "SOURCE" : "PUT", 0); > Ping. - Andreas
On Sun, Nov 10, 2019 at 05:07:29AM +0100, Andreas Rheinhardt wrote: > In case an AVBPrint was not complete, icecast_open() would free some > buffers that have not been allocated yet instead of freeing the data of > the AVBPrint (if they have been allocated). Because this error does not > trigger a jump to the general cleanup section any more, one can moreover > remove a (now unnecessary) initialization of a pointer. > > Furthermore, finalizing an AVBPrint can fail (namely when the string > inside the AVBPrint has not been allocated yet) and so this needs to be > checked. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/icecast.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) will apply thx [...]
Michael Niedermayer: > On Sun, Nov 10, 2019 at 05:07:29AM +0100, Andreas Rheinhardt wrote: >> In case an AVBPrint was not complete, icecast_open() would free some >> buffers that have not been allocated yet instead of freeing the data of >> the AVBPrint (if they have been allocated). Because this error does not >> trigger a jump to the general cleanup section any more, one can moreover >> remove a (now unnecessary) initialization of a pointer. >> >> Furthermore, finalizing an AVBPrint can fail (namely when the string >> inside the AVBPrint has not been allocated yet) and so this needs to be >> checked. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >> --- >> libavformat/icecast.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) > > will apply > > thx > > [...] > Thanks. Can you also take a look at the follow-up patch [1]? - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/252778.html
On 19 Dec 2019, at 23:36, Andreas Rheinhardt wrote: > Michael Niedermayer: >> On Sun, Nov 10, 2019 at 05:07:29AM +0100, Andreas Rheinhardt wrote: >>> In case an AVBPrint was not complete, icecast_open() would free some >>> buffers that have not been allocated yet instead of freeing the data of >>> the AVBPrint (if they have been allocated). Because this error does not >>> trigger a jump to the general cleanup section any more, one can moreover >>> remove a (now unnecessary) initialization of a pointer. >>> >>> Furthermore, finalizing an AVBPrint can fail (namely when the string >>> inside the AVBPrint has not been allocated yet) and so this needs to be >>> checked. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> >>> --- >>> libavformat/icecast.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> will apply >> >> thx >> >> [...] >> > Thanks. Can you also take a look at the follow-up patch [1]? > > - Andreas Sorry for the late reply, for some reason only saw this patch now… LGTM > > [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-November/252778.html > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/icecast.c b/libavformat/icecast.c index d2198b78ec..052cd37f3e 100644 --- a/libavformat/icecast.c +++ b/libavformat/icecast.c @@ -89,7 +89,7 @@ static int icecast_open(URLContext *h, const char *uri, int flags) // URI part variables char h_url[1024], host[1024], auth[1024], path[1024]; - char *headers = NULL, *user = NULL; + char *headers, *user = NULL; int port, ret; AVBPrint bp; @@ -105,10 +105,11 @@ static int icecast_open(URLContext *h, const char *uri, int flags) cat_header(&bp, "Ice-Genre", s->genre); cat_header(&bp, "Ice-Public", s->public ? "1" : "0"); if (!av_bprint_is_complete(&bp)) { - ret = AVERROR(ENOMEM); - goto cleanup; + av_bprint_finalize(&bp, NULL); + return AVERROR(ENOMEM); } - av_bprint_finalize(&bp, &headers); + if ((ret = av_bprint_finalize(&bp, &headers)) < 0) + return ret; // Set options av_dict_set(&opt_dict, "method", s->legacy_icecast ? "SOURCE" : "PUT", 0);
In case an AVBPrint was not complete, icecast_open() would free some buffers that have not been allocated yet instead of freeing the data of the AVBPrint (if they have been allocated). Because this error does not trigger a jump to the general cleanup section any more, one can moreover remove a (now unnecessary) initialization of a pointer. Furthermore, finalizing an AVBPrint can fail (namely when the string inside the AVBPrint has not been allocated yet) and so this needs to be checked. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/icecast.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)