| Message ID | AS8P250MB074479EF0D42070BC0EEB0658F25A@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | 652add3a6d675a568eb7564d85434712829b03d3 |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avcodec/cbs_h266: Remove double ; | 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 |
On 6/29/2023 3:19 PM, Andreas Rheinhardt wrote: > The discrepancy between the definition and the declaration > in allfilters.c is actually UB. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavfilter/vf_ccrepack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_ccrepack.c b/libavfilter/vf_ccrepack.c > index 61eb2128ae..950bb7b528 100644 > --- a/libavfilter/vf_ccrepack.c > +++ b/libavfilter/vf_ccrepack.c > @@ -92,7 +92,7 @@ static const AVFilterPad avfilter_vf_ccrepack_outputs[] = { > }, > }; > > -AVFilter ff_vf_ccrepack = { > +const AVFilter ff_vf_ccrepack = { > .name = "ccrepack", > .description = NULL_IF_CONFIG_SMALL("Repack CEA-708 closed caption metadata"), > .uninit = uninit, LGTM
On Thu, Jun 29, 2023 at 8:18 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > The discrepancy between the definition and the declaration > in allfilters.c is actually UB. > I get no such message with ubsan. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavfilter/vf_ccrepack.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavfilter/vf_ccrepack.c b/libavfilter/vf_ccrepack.c > index 61eb2128ae..950bb7b528 100644 > --- a/libavfilter/vf_ccrepack.c > +++ b/libavfilter/vf_ccrepack.c > @@ -92,7 +92,7 @@ static const AVFilterPad avfilter_vf_ccrepack_outputs[] > = { > }, > }; > > -AVFilter ff_vf_ccrepack = { > +const AVFilter ff_vf_ccrepack = { > .name = "ccrepack", > .description = NULL_IF_CONFIG_SMALL("Repack CEA-708 closed caption > metadata"), > .uninit = uninit, > -- > 2.34.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". >
Paul B Mahol: > On Thu, Jun 29, 2023 at 8:18 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> The discrepancy between the definition and the declaration >> in allfilters.c is actually UB. >> > > I get no such message with ubsan. > UBSan is a runtime UB-detector, not a compile-time UB detector. The earlier code is UB because of 6.2.7 (2) of C11: "All declarations that refer to the same object or function shall have compatible type; otherwise, the behavior is undefined." A type and its const-qualified type are not compatible. - Andreas
On Thu, Jun 29, 2023 at 9:35 PM Andreas Rheinhardt < andreas.rheinhardt@outlook.com> wrote: > Paul B Mahol: > > On Thu, Jun 29, 2023 at 8:18 PM Andreas Rheinhardt < > > andreas.rheinhardt@outlook.com> wrote: > > > >> The discrepancy between the definition and the declaration > >> in allfilters.c is actually UB. > >> > > > > I get no such message with ubsan. > > > > UBSan is a runtime UB-detector, not a compile-time UB detector. > The earlier code is UB because of 6.2.7 (2) of C11: "All declarations > that refer to the same object or function shall have compatible type; > otherwise, the behavior is undefined." A type and its const-qualified > type are not compatible. > This is so minor, that it is fully irrelevant. > > - Andreas > >
On 6/29/2023 4:42 PM, Paul B Mahol wrote: > On Thu, Jun 29, 2023 at 9:35 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Thu, Jun 29, 2023 at 8:18 PM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> The discrepancy between the definition and the declaration >>>> in allfilters.c is actually UB. >>>> >>> >>> I get no such message with ubsan. >>> >> >> UBSan is a runtime UB-detector, not a compile-time UB detector. >> The earlier code is UB because of 6.2.7 (2) of C11: "All declarations >> that refer to the same object or function shall have compatible type; >> otherwise, the behavior is undefined." A type and its const-qualified >> type are not compatible. >> > > This is so minor, that it is fully irrelevant. Msvc was pedantic enough to complain about a double colon, so who knows if some compiler would do the same for this. If the spec states both must match, then adding a "const" is hardly a problem.
Paul B Mahol: > On Thu, Jun 29, 2023 at 9:35 PM Andreas Rheinhardt < > andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >>> On Thu, Jun 29, 2023 at 8:18 PM Andreas Rheinhardt < >>> andreas.rheinhardt@outlook.com> wrote: >>> >>>> The discrepancy between the definition and the declaration >>>> in allfilters.c is actually UB. >>>> >>> >>> I get no such message with ubsan. >>> >> >> UBSan is a runtime UB-detector, not a compile-time UB detector. >> The earlier code is UB because of 6.2.7 (2) of C11: "All declarations >> that refer to the same object or function shall have compatible type; >> otherwise, the behavior is undefined." A type and its const-qualified >> type are not compatible. >> > > This is so minor, that it is fully irrelevant. > The actual advantage of these patches is that the objects can be put into read-only memory (.data.rel.ro in elf). - Andreas
Le 29 juin 2023 22:42:17 GMT+03:00, Paul B Mahol <onemda@gmail.com> a écrit : >On Thu, Jun 29, 2023 at 9:35 PM Andreas Rheinhardt < >andreas.rheinhardt@outlook.com> wrote: > >> Paul B Mahol: >> > On Thu, Jun 29, 2023 at 8:18 PM Andreas Rheinhardt < >> > andreas.rheinhardt@outlook.com> wrote: >> > >> >> The discrepancy between the definition and the declaration >> >> in allfilters.c is actually UB. >> >> >> > >> > I get no such message with ubsan. >> > >> >> UBSan is a runtime UB-detector, not a compile-time UB detector. >> The earlier code is UB because of 6.2.7 (2) of C11: "All declarations >> that refer to the same object or function shall have compatible type; >> otherwise, the behavior is undefined." A type and its const-qualified >> type are not compatible. >> > >This is so minor, that it is fully irrelevant. UB is one of the most severe type of bug that can happen in C. How exactly is that "fully irrelevant"? Nobody is ordering you to fix this bug if you don't want to. That's not a reason to block an objective simple well-understood and well-informed bug fix that somebody else made. > > >> >> - 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".
diff --git a/libavfilter/vf_ccrepack.c b/libavfilter/vf_ccrepack.c index 61eb2128ae..950bb7b528 100644 --- a/libavfilter/vf_ccrepack.c +++ b/libavfilter/vf_ccrepack.c @@ -92,7 +92,7 @@ static const AVFilterPad avfilter_vf_ccrepack_outputs[] = { }, }; -AVFilter ff_vf_ccrepack = { +const AVFilter ff_vf_ccrepack = { .name = "ccrepack", .description = NULL_IF_CONFIG_SMALL("Repack CEA-708 closed caption metadata"), .uninit = uninit,
The discrepancy between the definition and the declaration in allfilters.c is actually UB. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavfilter/vf_ccrepack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)