| Message ID | 20230310134547.16970-1-anton@khirnov.net |
|---|---|
| State | Accepted |
| Commit | 1e406692e5a1e6d9ca852f746088a5610ff80d14 |
| Headers | show |
| Series | [FFmpeg-devel] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > Sent: 2023年3月10日 21:46 > To: ffmpeg-devel@ffmpeg.org > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing > > Fixes #10243 > --- > fftools/ffmpeg_mux_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c > index b3cc502fdd..09d24ba8e5 100644 > --- a/fftools/ffmpeg_mux_init.c > +++ b/fftools/ffmpeg_mux_init.c > @@ -2063,7 +2063,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux, > if (next) > *next++ = 0; > > - if (!memcmp(p, "chapters", 8)) { > + if (strstr(p, "chapters") == p) { Does strncmp() more efficient in this case? > AVChapter * const *ch = mux->fc->chapters; > unsigned int nb_ch = mux->fc->nb_chapters; > int j; > -- > 2.39.1 > > _______________________________________________ > 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".
Quoting Zhao Zhili (2023-03-10 15:44:56) > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > > Sent: 2023年3月10日 21:46 > > To: ffmpeg-devel@ffmpeg.org > > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing > > > > Fixes #10243 > > --- > > fftools/ffmpeg_mux_init.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c > > index b3cc502fdd..09d24ba8e5 100644 > > --- a/fftools/ffmpeg_mux_init.c > > +++ b/fftools/ffmpeg_mux_init.c > > @@ -2063,7 +2063,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux, > > if (next) > > *next++ = 0; > > > > - if (!memcmp(p, "chapters", 8)) { > > + if (strstr(p, "chapters") == p) { > > Does strncmp() more efficient in this case? I don't see the point of optimizing this code for speed. A strncmp call is longer and less readable IMO.
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > Sent: 2023年3月11日 18:37 > To: 'FFmpeg development discussions and patches' <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing > > Quoting Zhao Zhili (2023-03-10 15:44:56) > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > > > Sent: 2023年3月10日 21:46 > > > To: ffmpeg-devel@ffmpeg.org > > > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing > > > > > > Fixes #10243 > > > --- > > > fftools/ffmpeg_mux_init.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c > > > index b3cc502fdd..09d24ba8e5 100644 > > > --- a/fftools/ffmpeg_mux_init.c > > > +++ b/fftools/ffmpeg_mux_init.c > > > @@ -2063,7 +2063,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux, > > > if (next) > > > *next++ = 0; > > > > > > - if (!memcmp(p, "chapters", 8)) { > > > + if (strstr(p, "chapters") == p) { > > > > Does strncmp() more efficient in this case? > > I don't see the point of optimizing this code for speed. A strncmp call > is longer and less readable IMO. This is a case for the need of a strstarts(). strncmp is more intuitive than strstr() for this job. https://github.com/torvalds/linux/blob/master/include/linux/string.h#L215 static inline bool strstarts(const char *str, const char *prefix) { return strncmp(str, prefix, strlen(prefix)) == 0; } > > -- > Anton Khirnov > _______________________________________________ > 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".
Quoting Zhao Zhili (2023-03-11 12:45:30) > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > > Sent: 2023年3月11日 18:37 > > To: 'FFmpeg development discussions and patches' <ffmpeg-devel@ffmpeg.org> > > Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing > > > > Quoting Zhao Zhili (2023-03-10 15:44:56) > > > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Anton Khirnov > > > > Sent: 2023年3月10日 21:46 > > > > To: ffmpeg-devel@ffmpeg.org > > > > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: avoid invalid reads in forced keyframe parsing > > > > > > > > Fixes #10243 > > > > --- > > > > fftools/ffmpeg_mux_init.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c > > > > index b3cc502fdd..09d24ba8e5 100644 > > > > --- a/fftools/ffmpeg_mux_init.c > > > > +++ b/fftools/ffmpeg_mux_init.c > > > > @@ -2063,7 +2063,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux, > > > > if (next) > > > > *next++ = 0; > > > > > > > > - if (!memcmp(p, "chapters", 8)) { > > > > + if (strstr(p, "chapters") == p) { > > > > > > Does strncmp() more efficient in this case? > > > > I don't see the point of optimizing this code for speed. A strncmp call > > is longer and less readable IMO. > > This is a case for the need of a strstarts(). strncmp is more intuitive than strstr() > for this job. Yes, having a "string starts with" function in stdlib would have been nice. But as it's not there, we have to make do with what we have.
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c index b3cc502fdd..09d24ba8e5 100644 --- a/fftools/ffmpeg_mux_init.c +++ b/fftools/ffmpeg_mux_init.c @@ -2063,7 +2063,7 @@ static void parse_forced_key_frames(KeyframeForceCtx *kf, const Muxer *mux, if (next) *next++ = 0; - if (!memcmp(p, "chapters", 8)) { + if (strstr(p, "chapters") == p) { AVChapter * const *ch = mux->fc->chapters; unsigned int nb_ch = mux->fc->nb_chapters; int j;