| Message ID | 1577028418-15051-1-git-send-email-zhongli_dev@126.com |
|---|---|
| State | New |
| Headers | show |
On 12/22/2019 12:26 PM, Zhong Li wrote: > Signed-off-by: Zhong Li <zhongli_dev@126.com> > --- > libavcodec/midivid.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > index 38465c5..3dac3f1 100644 > --- a/libavcodec/midivid.c > +++ b/libavcodec/midivid.c > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame *frame) > if (intra_flag) { > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > } else { > - int skip_linesize; > + int ret, skip_linesize; > > nb_blocks = bytestream2_get_le32(gb); > skip_linesize = avctx->width >> 1; > mask_start = gb->buffer_start + bytestream2_tell(gb); > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > - if (bytestream2_get_bytes_left(gb) < mask_size) > + ret = bytestream2_get_bytes_left(gb); > + if (ret < mask_size) What is this fixing? > return AVERROR_INVALIDDATA; > > - init_get_bits8(&mask, mask_start, mask_size); > + ret = init_get_bits8(&mask, mask_start, mask_size); > + if (ret < 0) > + return AVERROR_INVALIDDATA; > bytestream2_skip(gb, mask_size); > skip = s->skip; > >
On Sun, Dec 22, 2019 at 03:26:58PM +0000, Zhong Li wrote: > Signed-off-by: Zhong Li <zhongli_dev@126.com> > --- > libavcodec/midivid.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > index 38465c5..3dac3f1 100644 > --- a/libavcodec/midivid.c > +++ b/libavcodec/midivid.c > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame *frame) > if (intra_flag) { > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > } else { > - int skip_linesize; > + int ret, skip_linesize; > > nb_blocks = bytestream2_get_le32(gb); > skip_linesize = avctx->width >> 1; > mask_start = gb->buffer_start + bytestream2_tell(gb); > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > - if (bytestream2_get_bytes_left(gb) < mask_size) > + ret = bytestream2_get_bytes_left(gb); > + if (ret < mask_size) > return AVERROR_INVALIDDATA; don't need change it I think. > > - init_get_bits8(&mask, mask_start, mask_size); > + ret = init_get_bits8(&mask, mask_start, mask_size); > + if (ret < 0) > + return AVERROR_INVALIDDATA; return ret. That's why use ret. > bytestream2_skip(gb, mask_size); > skip = s->skip; > > -- > 1.8.3.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".
On Mon, Dec 23, 2019 at 1:53 AM Limin Wang <lance.lmwang@gmail.com> wrote: > On Sun, Dec 22, 2019 at 03:26:58PM +0000, Zhong Li wrote: > > Signed-off-by: Zhong Li <zhongli_dev@126.com> > > --- > > libavcodec/midivid.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > > index 38465c5..3dac3f1 100644 > > --- a/libavcodec/midivid.c > > +++ b/libavcodec/midivid.c > > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, > AVCodecContext *avctx, AVFrame *frame) > > if (intra_flag) { > > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > > } else { > > - int skip_linesize; > > + int ret, skip_linesize; > > > > nb_blocks = bytestream2_get_le32(gb); > > skip_linesize = avctx->width >> 1; > > mask_start = gb->buffer_start + bytestream2_tell(gb); > > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > > > - if (bytestream2_get_bytes_left(gb) < mask_size) > > + ret = bytestream2_get_bytes_left(gb); > > + if (ret < mask_size) > > return AVERROR_INVALIDDATA; > > don't need change it I think. > > > > > - init_get_bits8(&mask, mask_start, mask_size); > > + ret = init_get_bits8(&mask, mask_start, mask_size); > > + if (ret < 0) > > + return AVERROR_INVALIDDATA; > > return ret. That's why use ret. > > But you are not returning ret. - Andreas
On Mon, Dec 23, 2019 at 02:00:10AM +0100, Andreas Rheinhardt wrote: > On Mon, Dec 23, 2019 at 1:53 AM Limin Wang <lance.lmwang@gmail.com> wrote: > > > On Sun, Dec 22, 2019 at 03:26:58PM +0000, Zhong Li wrote: > > > Signed-off-by: Zhong Li <zhongli_dev@126.com> > > > --- > > > libavcodec/midivid.c | 9 ++++++--- > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > > > index 38465c5..3dac3f1 100644 > > > --- a/libavcodec/midivid.c > > > +++ b/libavcodec/midivid.c > > > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, > > AVCodecContext *avctx, AVFrame *frame) > > > if (intra_flag) { > > > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > > > } else { > > > - int skip_linesize; > > > + int ret, skip_linesize; > > > > > > nb_blocks = bytestream2_get_le32(gb); > > > skip_linesize = avctx->width >> 1; > > > mask_start = gb->buffer_start + bytestream2_tell(gb); > > > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > > > > > - if (bytestream2_get_bytes_left(gb) < mask_size) > > > + ret = bytestream2_get_bytes_left(gb); > > > + if (ret < mask_size) > > > return AVERROR_INVALIDDATA; > > > > don't need change it I think. > > > > > > > > - init_get_bits8(&mask, mask_start, mask_size); > > > + ret = init_get_bits8(&mask, mask_start, mask_size); > > > + if (ret < 0) > > > + return AVERROR_INVALIDDATA; > > > > return ret. That's why use ret. > > > > > But you are not returning ret. But It's not my code, I'm a code reviewer. > > - Andreas > _______________________________________________ > 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".
On Mon, Dec 23, 2019 at 6:03 AM Limin Wang <lance.lmwang@gmail.com> wrote: > On Mon, Dec 23, 2019 at 02:00:10AM +0100, Andreas Rheinhardt wrote: > > On Mon, Dec 23, 2019 at 1:53 AM Limin Wang <lance.lmwang@gmail.com> > wrote: > > > > > On Sun, Dec 22, 2019 at 03:26:58PM +0000, Zhong Li wrote: > > > > Signed-off-by: Zhong Li <zhongli_dev@126.com> > > > > --- > > > > libavcodec/midivid.c | 9 ++++++--- > > > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > > > > index 38465c5..3dac3f1 100644 > > > > --- a/libavcodec/midivid.c > > > > +++ b/libavcodec/midivid.c > > > > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, > > > AVCodecContext *avctx, AVFrame *frame) > > > > if (intra_flag) { > > > > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > > > > } else { > > > > - int skip_linesize; > > > > + int ret, skip_linesize; > > > > > > > > nb_blocks = bytestream2_get_le32(gb); > > > > skip_linesize = avctx->width >> 1; > > > > mask_start = gb->buffer_start + bytestream2_tell(gb); > > > > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > > > > > > > - if (bytestream2_get_bytes_left(gb) < mask_size) > > > > + ret = bytestream2_get_bytes_left(gb); > > > > + if (ret < mask_size) > > > > return AVERROR_INVALIDDATA; > > > > > > don't need change it I think. > > > > > > > > > > > - init_get_bits8(&mask, mask_start, mask_size); > > > > + ret = init_get_bits8(&mask, mask_start, mask_size); > > > > + if (ret < 0) > > > > + return AVERROR_INVALIDDATA; > > > > > > return ret. That's why use ret. > > > > > > > > But you are not returning ret. > > But It's not my code, I'm a code reviewer. > > You are right. Forget my previous mail, it's just nonsense then. - Andreas
Limin Wang (12019-12-23):
> But It's not my code, I'm a code reviewer.
Then review that ret needs to be returned ;-) It needs more change, not
less.
And the commit message needs to be explicit. CID may not be available
everywhere or everywhen.
Regards,
James Almer <jamrial@gmail.com> 于2019年12月22日周日 下午11:29写道： > > On 12/22/2019 12:26 PM, Zhong Li wrote: > > Signed-off-by: Zhong Li <zhongli_dev@126.com> > > --- > > libavcodec/midivid.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > > index 38465c5..3dac3f1 100644 > > --- a/libavcodec/midivid.c > > +++ b/libavcodec/midivid.c > > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame *frame) > > if (intra_flag) { > > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > > } else { > > - int skip_linesize; > > + int ret, skip_linesize; > > > > nb_blocks = bytestream2_get_le32(gb); > > skip_linesize = avctx->width >> 1; > > mask_start = gb->buffer_start + bytestream2_tell(gb); > > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > > > - if (bytestream2_get_bytes_left(gb) < mask_size) > > + ret = bytestream2_get_bytes_left(gb); > > + if (ret < mask_size) > > What is this fixing? Not fix. Just want to use same coding stytle as following.
Nicolas George <george@nsup.org> 于2019年12月23日周一 下午5:00写道： > > Limin Wang (12019-12-23): > > But It's not my code, I'm a code reviewer. > > Then review that ret needs to be returned ;-) It needs more change, not > less. Could specify what "more change" is needed?
Zhong Li (12019-12-23): > > Then review that ret needs to be returned ;-) It needs more change, not ^^^^^^^^^^^^^^^^^^^^^^^^ > > less. > > Could specify what "more change" is needed? See above, I underlined it. Regards,
On 12/22/19, Zhong Li <zhongli_dev@126.com> wrote: > Signed-off-by: Zhong Li <zhongli_dev@126.com> > --- > libavcodec/midivid.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > index 38465c5..3dac3f1 100644 > --- a/libavcodec/midivid.c > +++ b/libavcodec/midivid.c > @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext > *avctx, AVFrame *frame) > if (intra_flag) { > nb_blocks = (avctx->width / 2) * (avctx->height / 2); > } else { > - int skip_linesize; > + int ret, skip_linesize; > > nb_blocks = bytestream2_get_le32(gb); > skip_linesize = avctx->width >> 1; > mask_start = gb->buffer_start + bytestream2_tell(gb); > mask_size = (avctx->width >> 5) * (avctx->height >> 2); > > - if (bytestream2_get_bytes_left(gb) < mask_size) > + ret = bytestream2_get_bytes_left(gb); > + if (ret < mask_size) > return AVERROR_INVALIDDATA; Pointless > > - init_get_bits8(&mask, mask_start, mask_size); > + ret = init_get_bits8(&mask, mask_start, mask_size); > + if (ret < 0) > + return AVERROR_INVALIDDATA; ret should be returned here > bytestream2_skip(gb, mask_size); > skip = s->skip; > > -- > 1.8.3.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".
On 12/23/2019 9:31 AM, Zhong Li wrote: > James Almer <jamrial@gmail.com> 于2019年12月22日周日 下午11:29写道： >> >> On 12/22/2019 12:26 PM, Zhong Li wrote: >>> Signed-off-by: Zhong Li <zhongli_dev@126.com> >>> --- >>> libavcodec/midivid.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c >>> index 38465c5..3dac3f1 100644 >>> --- a/libavcodec/midivid.c >>> +++ b/libavcodec/midivid.c >>> @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame *frame) >>> if (intra_flag) { >>> nb_blocks = (avctx->width / 2) * (avctx->height / 2); >>> } else { >>> - int skip_linesize; >>> + int ret, skip_linesize; >>> >>> nb_blocks = bytestream2_get_le32(gb); >>> skip_linesize = avctx->width >> 1; >>> mask_start = gb->buffer_start + bytestream2_tell(gb); >>> mask_size = (avctx->width >> 5) * (avctx->height >> 2); >>> >>> - if (bytestream2_get_bytes_left(gb) < mask_size) >>> + ret = bytestream2_get_bytes_left(gb); >>> + if (ret < mask_size) >> >> What is this fixing? > > Not fix. Just want to use same coding stytle as following. Unless you need it to return a value, then no point in making this change. Add the init_get_bits8() check only, and make it return ret as others suggested. Also, the commit subject should be something like "lavc/midivid: check return value of init_get_bits8()", and the CID 1456088 comment added further down in the commit message.
James Almer <jamrial@gmail.com> 于2019年12月23日周一 下午9:09写道： > > On 12/23/2019 9:31 AM, Zhong Li wrote: > > James Almer <jamrial@gmail.com> 于2019年12月22日周日 下午11:29写道： > >> > >> On 12/22/2019 12:26 PM, Zhong Li wrote: > >>> Signed-off-by: Zhong Li <zhongli_dev@126.com> > >>> --- > >>> libavcodec/midivid.c | 9 ++++++--- > >>> 1 file changed, 6 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c > >>> index 38465c5..3dac3f1 100644 > >>> --- a/libavcodec/midivid.c > >>> +++ b/libavcodec/midivid.c > >>> @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame *frame) > >>> if (intra_flag) { > >>> nb_blocks = (avctx->width / 2) * (avctx->height / 2); > >>> } else { > >>> - int skip_linesize; > >>> + int ret, skip_linesize; > >>> > >>> nb_blocks = bytestream2_get_le32(gb); > >>> skip_linesize = avctx->width >> 1; > >>> mask_start = gb->buffer_start + bytestream2_tell(gb); > >>> mask_size = (avctx->width >> 5) * (avctx->height >> 2); > >>> > >>> - if (bytestream2_get_bytes_left(gb) < mask_size) > >>> + ret = bytestream2_get_bytes_left(gb); > >>> + if (ret < mask_size) > >> > >> What is this fixing? > > > > Not fix. Just want to use same coding stytle as following. > > Unless you need it to return a value, then no point in making this change. > > Add the init_get_bits8() check only, and make it return ret as others > suggested. Also, the commit subject should be something like > "lavc/midivid: check return value of init_get_bits8()", and the CID > 1456088 comment added further down in the commit message. Yeah, thanks for comments. Will update.
diff --git a/libavcodec/midivid.c b/libavcodec/midivid.c index 38465c5..3dac3f1 100644 --- a/libavcodec/midivid.c +++ b/libavcodec/midivid.c @@ -63,17 +63,20 @@ static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame *frame) if (intra_flag) { nb_blocks = (avctx->width / 2) * (avctx->height / 2); } else { - int skip_linesize; + int ret, skip_linesize; nb_blocks = bytestream2_get_le32(gb); skip_linesize = avctx->width >> 1; mask_start = gb->buffer_start + bytestream2_tell(gb); mask_size = (avctx->width >> 5) * (avctx->height >> 2); - if (bytestream2_get_bytes_left(gb) < mask_size) + ret = bytestream2_get_bytes_left(gb); + if (ret < mask_size) return AVERROR_INVALIDDATA; - init_get_bits8(&mask, mask_start, mask_size); + ret = init_get_bits8(&mask, mask_start, mask_size); + if (ret < 0) + return AVERROR_INVALIDDATA; bytestream2_skip(gb, mask_size); skip = s->skip;
Signed-off-by: Zhong Li <zhongli_dev@126.com> --- libavcodec/midivid.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)