Andreas Rheinhardt (12020-08-23): > This happened in parse_link_name() if there was a '[' without matching > ']'. While this is not undefined behaviour (pointer arithmetic one > beyond the end of an array works fine as long as there are no accesses), > it is potentially dangerous. It currently isn't (all callers of > parse_link_name() treat this as an error and don't access the string any > more), but making sure that this will never cause trouble in the future > seems nevertheless worthwhile. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > --- > libavfilter/graphparser.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/libavfilter/graphparser.c b/libavfilter/graphparser.c > index dfb94788e1..e96b20418e 100644 > --- a/libavfilter/graphparser.c > +++ b/libavfilter/graphparser.c > @@ -63,7 +63,7 @@ static char *parse_link_name(const char **buf, void > *log_ctx) > > name = av_get_token(buf, "]"); > if (!name) > - goto fail; > + return NULL;
This looks ok.
>
> if (!name[0]) {
> av_log(log_ctx, AV_LOG_ERROR,
> @@ -71,12 +71,14 @@ static char *parse_link_name(const char **buf, void
> *log_ctx)
> goto fail;
> }
>
> - if (*(*buf)++ != ']') {
> + if (**buf != ']') {
> av_log(log_ctx, AV_LOG_ERROR,
> "Mismatched '[' found in the following: \"%s\".\n", start);
> fail:
> av_freep(&name);
> + return NULL;
> }
> + (*buf)++;
>
> return name;
> }
I would like it better if you took the opportunity to get rid of this
spaghetti goto. A fail label in an if that uses the side effect of
av_freep() to make sure the final return will return NULL? Brr, that's
bound to come back and bite us at some time.
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
_______________________________________________ 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".
