| Message ID | 20240506012307.807589-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | a68aa951b21b8b7db0a5200bcfebc0a077a5f094 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/h264_slice: Remove dead sps check | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, May 06, 2024 at 03:23:07AM +0200, Michael Niedermayer wrote: > Fixes: CID1439574 Dereference after null check > > Sponsored-by: Sovereign Tech Fund > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/h264_slice.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...]
On Mon, 13 May 2024, 02:32 Michael Niedermayer, <michael@niedermayer.cc> wrote: > On Mon, May 06, 2024 at 03:23:07AM +0200, Michael Niedermayer wrote: > > Fixes: CID1439574 Dereference after null check > > > > Sponsored-by: Sovereign Tech Fund > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/h264_slice.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > will apply > I don't understand what this fixes >
Le 6 mai 2024 04:23:07 GMT+03:00, Michael Niedermayer <michael@niedermayer.cc> a écrit : >Fixes: CID1439574 Dereference after null check If SPS is guaranteed to be non-NULL, there should probably be an assertion to document it, and it should be *before* that alleged dereference (which is not visible in the patch context). > >Sponsored-by: Sovereign Tech Fund >Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >--- > libavcodec/h264_slice.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c >index 90d37f60848..ce2c4caca1b 100644 >--- a/libavcodec/h264_slice.c >+++ b/libavcodec/h264_slice.c >@@ -1396,7 +1396,7 @@ static int h264_field_start(H264Context *h, const H264SliceContext *sl, > > sps = h->ps.sps; > >- if (sps && sps->bitstream_restriction_flag && >+ if (sps->bitstream_restriction_flag && > h->avctx->has_b_frames < sps->num_reorder_frames) { > h->avctx->has_b_frames = sps->num_reorder_frames; > }
On Mon, May 13, 2024 at 09:04:34AM +0300, Rémi Denis-Courmont wrote: > > > Le 6 mai 2024 04:23:07 GMT+03:00, Michael Niedermayer <michael@niedermayer.cc> a écrit : > >Fixes: CID1439574 Dereference after null check > > If SPS is guaranteed to be non-NULL, there should probably be an assertion to document it, and it should be *before* that alleged dereference (which is not visible in the patch context). why ? I mean why would sps require an assert*(sps) but the overwhelming majority of pointers, we just dereference without assert before them ? thx [...]
On Mon, May 13, 2024 at 06:15:16AM +0100, Kieran Kunhya wrote: > On Mon, 13 May 2024, 02:32 Michael Niedermayer, <michael@niedermayer.cc> > wrote: > > > On Mon, May 06, 2024 at 03:23:07AM +0200, Michael Niedermayer wrote: > > > Fixes: CID1439574 Dereference after null check > > > > > > Sponsored-by: Sovereign Tech Fund > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > --- > > > libavcodec/h264_slice.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > will apply > > > > I don't understand what this fixes it is not possible for sps to be NULL in this function (see line 1424) thanks [...]
On 5/13/2024 7:49 PM, Michael Niedermayer wrote: > On Mon, May 13, 2024 at 06:15:16AM +0100, Kieran Kunhya wrote: >> On Mon, 13 May 2024, 02:32 Michael Niedermayer, <michael@niedermayer.cc> >> wrote: >> >>> On Mon, May 06, 2024 at 03:23:07AM +0200, Michael Niedermayer wrote: >>>> Fixes: CID1439574 Dereference after null check >>>> >>>> Sponsored-by: Sovereign Tech Fund >>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>>> --- >>>> libavcodec/h264_slice.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> will apply >>> >> >> I don't understand what this fixes > > it is not possible for sps to be NULL in this function (see line 1424) It's also passed to ff_h264_init_poc(), which expects it to not be NULL. In any case, when the coverity issues are about something that's not a bug but just noise like superfluous checks, might as well use something like "closes" rather than "fixes".
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 90d37f60848..ce2c4caca1b 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -1396,7 +1396,7 @@ static int h264_field_start(H264Context *h, const H264SliceContext *sl, sps = h->ps.sps; - if (sps && sps->bitstream_restriction_flag && + if (sps->bitstream_restriction_flag && h->avctx->has_b_frames < sps->num_reorder_frames) { h->avctx->has_b_frames = sps->num_reorder_frames; }
Fixes: CID1439574 Dereference after null check Sponsored-by: Sovereign Tech Fund Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/h264_slice.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)