| Message ID | 20190717032940.48332-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | ab4795a085cd3deacb5e4bbaf527d66171361024 |
| Headers | show |
On 7/17/19, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > Unknown-length elements end when an element not allowed in them, but > allowed at a higher level is encountered. In order to check for this, > c1abd95a added a pointer to every syntax level's parent to each > EbmlSyntax. Given that the parent must of course also reference the > child in order to be able to enter said child level, one needs to use > forward declarations. > These forward declarations constitute tentative definitions and tentative > definitions with internal linkage (like our syntaxes) must not be an > incomplete type. Yet they were an incomplete type and while GCC and > Clang did not even warn about this (on default warning levels), it > broke compilation with MSVC. Therefore this commit adds the sizes. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > 1. I am terribly sorry for this. I did test with both GCC and Clang, but > not with -pedantic, but only with default warning levels. > 2. I don't have an MSVC setup here, so could please someone confirm that > this patch indeed fixes the compilation? All I know is that GCC's > -pedantic warnings are gone with this patch and that the requirement > that tentative definitions with internal linkage must not be of > incomplete type is fulfilled now. > > libavformat/matroskadec.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index b97189e674..fb0356e7b7 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -389,12 +389,16 @@ typedef struct MatroskaDemuxContext { > > #define CHILD_OF(parent) { .def = { .n = parent } } > > -static const EbmlSyntax ebml_syntax[], matroska_segment[], > matroska_track_video_color[], matroska_track_video[], > - matroska_track[], matroska_track_encoding[], > matroska_track_encodings[], > - matroska_track_combine_planes[], > matroska_track_operation[], matroska_tracks[], > - matroska_attachments[], matroska_chapter_entry[], > matroska_chapter[], matroska_chapters[], > - matroska_index_entry[], matroska_index[], > matroska_tag[], matroska_tags[], matroska_seekhead[], > - matroska_blockadditions[], matroska_blockgroup[], > matroska_cluster_parsing[]; > +// The following forward declarations need their size because > +// a tentative definition with internal linkage must not be an > +// incomplete type (6.7.2 in C90, 6.9.2 in C99). > +// Removing the sizes breaks MSVC. > +static const EbmlSyntax ebml_syntax[3], matroska_segment[9], > matroska_track_video_color[15], matroska_track_video[19], > + matroska_track[27], matroska_track_encoding[6], > matroska_track_encodings[2], > + matroska_track_combine_planes[2], > matroska_track_operation[2], matroska_tracks[2], > + matroska_attachments[2], matroska_chapter_entry[9], > matroska_chapter[6], matroska_chapters[2], > + matroska_index_entry[3], matroska_index[2], > matroska_tag[3], matroska_tags[2], matroska_seekhead[2], > + matroska_blockadditions[2], matroska_blockgroup[8], > matroska_cluster_parsing[8]; > > static const EbmlSyntax ebml_header[] = { > { EBML_ID_EBMLREADVERSION, EBML_UINT, 0, offsetof(Ebml, version), > { .u = EBML_VERSION } }, > -- > 2.21.0 > Looks fine. > _______________________________________________ > 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 Wed, Jul 17, 2019 at 5:35 AM Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > > Unknown-length elements end when an element not allowed in them, but > allowed at a higher level is encountered. In order to check for this, > c1abd95a added a pointer to every syntax level's parent to each > EbmlSyntax. Given that the parent must of course also reference the > child in order to be able to enter said child level, one needs to use > forward declarations. > These forward declarations constitute tentative definitions and tentative > definitions with internal linkage (like our syntaxes) must not be an > incomplete type. Yet they were an incomplete type and while GCC and > Clang did not even warn about this (on default warning levels), it > broke compilation with MSVC. Therefore this commit adds the sizes. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > 1. I am terribly sorry for this. I did test with both GCC and Clang, but > not with -pedantic, but only with default warning levels. > 2. I don't have an MSVC setup here, so could please someone confirm that > this patch indeed fixes the compilation? All I know is that GCC's > -pedantic warnings are gone with this patch and that the requirement > that tentative definitions with internal linkage must not be of > incomplete type is fulfilled now. > > libavformat/matroskadec.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c > index b97189e674..fb0356e7b7 100644 > --- a/libavformat/matroskadec.c > +++ b/libavformat/matroskadec.c > @@ -389,12 +389,16 @@ typedef struct MatroskaDemuxContext { > > #define CHILD_OF(parent) { .def = { .n = parent } } > > -static const EbmlSyntax ebml_syntax[], matroska_segment[], matroska_track_video_color[], matroska_track_video[], > - matroska_track[], matroska_track_encoding[], matroska_track_encodings[], > - matroska_track_combine_planes[], matroska_track_operation[], matroska_tracks[], > - matroska_attachments[], matroska_chapter_entry[], matroska_chapter[], matroska_chapters[], > - matroska_index_entry[], matroska_index[], matroska_tag[], matroska_tags[], matroska_seekhead[], > - matroska_blockadditions[], matroska_blockgroup[], matroska_cluster_parsing[]; > +// The following forward declarations need their size because > +// a tentative definition with internal linkage must not be an > +// incomplete type (6.7.2 in C90, 6.9.2 in C99). > +// Removing the sizes breaks MSVC. > +static const EbmlSyntax ebml_syntax[3], matroska_segment[9], matroska_track_video_color[15], matroska_track_video[19], > + matroska_track[27], matroska_track_encoding[6], matroska_track_encodings[2], > + matroska_track_combine_planes[2], matroska_track_operation[2], matroska_tracks[2], > + matroska_attachments[2], matroska_chapter_entry[9], matroska_chapter[6], matroska_chapters[2], > + matroska_index_entry[3], matroska_index[2], matroska_tag[3], matroska_tags[2], matroska_seekhead[2], > + matroska_blockadditions[2], matroska_blockgroup[8], matroska_cluster_parsing[8]; > > static const EbmlSyntax ebml_header[] = { > { EBML_ID_EBMLREADVERSION, EBML_UINT, 0, offsetof(Ebml, version), { .u = EBML_VERSION } }, Fixes compilaton on MSVC and FATE passes. Will apply soon if noone beats me to it. As an additional note, this section still causes a lot of warnings because "const" is repeated. The typedef for EbmlSyntax is already const, so this evaluates to "const const" and makes it throw warnings about that. Maybe you can remove one of those in a follow up? - Hendrik
diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c index b97189e674..fb0356e7b7 100644 --- a/libavformat/matroskadec.c +++ b/libavformat/matroskadec.c @@ -389,12 +389,16 @@ typedef struct MatroskaDemuxContext { #define CHILD_OF(parent) { .def = { .n = parent } } -static const EbmlSyntax ebml_syntax[], matroska_segment[], matroska_track_video_color[], matroska_track_video[], - matroska_track[], matroska_track_encoding[], matroska_track_encodings[], - matroska_track_combine_planes[], matroska_track_operation[], matroska_tracks[], - matroska_attachments[], matroska_chapter_entry[], matroska_chapter[], matroska_chapters[], - matroska_index_entry[], matroska_index[], matroska_tag[], matroska_tags[], matroska_seekhead[], - matroska_blockadditions[], matroska_blockgroup[], matroska_cluster_parsing[]; +// The following forward declarations need their size because +// a tentative definition with internal linkage must not be an +// incomplete type (6.7.2 in C90, 6.9.2 in C99). +// Removing the sizes breaks MSVC. +static const EbmlSyntax ebml_syntax[3], matroska_segment[9], matroska_track_video_color[15], matroska_track_video[19], + matroska_track[27], matroska_track_encoding[6], matroska_track_encodings[2], + matroska_track_combine_planes[2], matroska_track_operation[2], matroska_tracks[2], + matroska_attachments[2], matroska_chapter_entry[9], matroska_chapter[6], matroska_chapters[2], + matroska_index_entry[3], matroska_index[2], matroska_tag[3], matroska_tags[2], matroska_seekhead[2], + matroska_blockadditions[2], matroska_blockgroup[8], matroska_cluster_parsing[8]; static const EbmlSyntax ebml_header[] = { { EBML_ID_EBMLREADVERSION, EBML_UINT, 0, offsetof(Ebml, version), { .u = EBML_VERSION } },
Unknown-length elements end when an element not allowed in them, but allowed at a higher level is encountered. In order to check for this, c1abd95a added a pointer to every syntax level's parent to each EbmlSyntax. Given that the parent must of course also reference the child in order to be able to enter said child level, one needs to use forward declarations. These forward declarations constitute tentative definitions and tentative definitions with internal linkage (like our syntaxes) must not be an incomplete type. Yet they were an incomplete type and while GCC and Clang did not even warn about this (on default warning levels), it broke compilation with MSVC. Therefore this commit adds the sizes. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- 1. I am terribly sorry for this. I did test with both GCC and Clang, but not with -pedantic, but only with default warning levels. 2. I don't have an MSVC setup here, so could please someone confirm that this patch indeed fixes the compilation? All I know is that GCC's -pedantic warnings are gone with this patch and that the requirement that tentative definitions with internal linkage must not be of incomplete type is fulfilled now. libavformat/matroskadec.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)