Message ID | 20220111204610.14262-2-anton@khirnov.net |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,01/35] lavu/fifo: disallow overly large fifo sizes | expand |
Anton Khirnov: > There should be no good reason for the callers to access any of its > contents. > > Define a new type for the internal struct that currently matches > AVFifoBuffer. This will allow adding new private fields without waiting > for the major bump and will be useful in the following commits. > > Unfortunately AVFifoBuffer fields cannot be marked as deprecated because > it would trigger a warning wherever fifo.h is #included, due to > inlined av_fifo_peek2(). > --- > doc/APIchanges | 4 ++++ > libavutil/fifo.c | 23 +++++++++++++++++++---- > libavutil/fifo.h | 14 ++++++++++++-- > libavutil/tests/fifo.c | 2 +- > libavutil/version.h | 1 + > 5 files changed, 37 insertions(+), 7 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 8df0364e4c..21fa02ae9d 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -14,6 +14,10 @@ libavutil: 2021-04-27 > > API changes, most recent first: > > +2022-01-xx - xxxxxxxxxx - lavu fifo.h > + Access to all AVFifoBuffer members is deprecated. The struct will > + become an incomplete type in a future major libavutil version. > + > 2022-01-04 - 78dc21b123e - lavu 57.16.100 - frame.h > Add AV_FRAME_DATA_DOVI_METADATA. > > diff --git a/libavutil/fifo.c b/libavutil/fifo.c > index f2f046b1f3..aaade01333 100644 > --- a/libavutil/fifo.c > +++ b/libavutil/fifo.c > @@ -28,9 +28,24 @@ > > #define FIFO_SIZE_MAX FFMIN3((uint64_t)INT_MAX, (uint64_t)UINT32_MAX, (uint64_t)SIZE_MAX) > > +#if FF_API_FIFO_PUBLIC > +# define CTX_STRUCT_NAME FifoBuffer > +#else > +# define CTX_STRUCT_NAME AVFifoBuffer This is invalid in pre-C11 (and will lead to compilation failures on old GCC versions): Pre-C11, typedefs were subject to the ODR, yet you already typedef AVFifoBuffer in fifo.h. > +#endif > + > +typedef struct CTX_STRUCT_NAME { > + // These fields must match then contents of AVFifoBuffer in fifo.h the contents > + // until FF_API_FIFO_PUBLIC is removed The actual spec-compliant way for this is to add an AVFifoBuffer at the start of this struct; it will also avoid the casts from FifoBuffer to AVFifoBuffer. This will make the accesses to it a bit more cumbersome and will mean more changes when FF_API_FIFO_PUBLIC is removed. (This would not be an issue if we required support for anonymous structs (mandatory in C11, supported by GCC and others long before that).) > + uint8_t *buffer; > + uint8_t *rptr, *wptr, *end; > + uint32_t rndx, wndx; > + ///////////////////////////////////////// > +} FifoBuffer; > + > AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size) > { > - AVFifoBuffer *f; > + FifoBuffer *f; > void *buffer; > > if (nmemb > FIFO_SIZE_MAX / size) > @@ -39,15 +54,15 @@ AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size) > buffer = av_realloc_array(NULL, nmemb, size); > if (!buffer) > return NULL; > - f = av_mallocz(sizeof(AVFifoBuffer)); > + f = av_mallocz(sizeof(*f)); > if (!f) { > av_free(buffer); > return NULL; > } > f->buffer = buffer; > f->end = f->buffer + nmemb * size; > - av_fifo_reset(f); > - return f; > + av_fifo_reset((AVFifoBuffer*)f); > + return (AVFifoBuffer*)f; > } > > AVFifoBuffer *av_fifo_alloc(unsigned int size) > diff --git a/libavutil/fifo.h b/libavutil/fifo.h > index f4fd291e59..ca4e7fe060 100644 > --- a/libavutil/fifo.h > +++ b/libavutil/fifo.h > @@ -28,11 +28,21 @@ > #include "avutil.h" > #include "attributes.h" > > -typedef struct AVFifoBuffer { > +#if FF_API_FIFO_PUBLIC > +/** > + * The contents of the struct are private and should not be accessed by the > + * callers in any way. > + */ > +#endif > +typedef struct AVFifoBuffer > +#if FF_API_FIFO_PUBLIC > +{ > uint8_t *buffer; > uint8_t *rptr, *wptr, *end; > uint32_t rndx, wndx; > -} AVFifoBuffer; > +} > +#endif > +AVFifoBuffer; > > /** > * Initialize an AVFifoBuffer. > diff --git a/libavutil/tests/fifo.c b/libavutil/tests/fifo.c > index a17d913233..e5aa88d252 100644 > --- a/libavutil/tests/fifo.c > +++ b/libavutil/tests/fifo.c > @@ -18,7 +18,7 @@ > > #include <stdio.h> > #include <stdlib.h> > -#include "libavutil/fifo.h" > +#include "libavutil/fifo.c" > > int main(void) > { > diff --git a/libavutil/version.h b/libavutil/version.h > index 953aac9d94..7c031f547e 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -110,6 +110,7 @@ > #define FF_API_COLORSPACE_NAME (LIBAVUTIL_VERSION_MAJOR < 58) > #define FF_API_AV_MALLOCZ_ARRAY (LIBAVUTIL_VERSION_MAJOR < 58) > #define FF_API_FIFO_PEEK2 (LIBAVUTIL_VERSION_MAJOR < 58) > +#define FF_API_FIFO_PUBLIC (LIBAVUTIL_VERSION_MAJOR < 58) > > /** > * @} >
diff --git a/doc/APIchanges b/doc/APIchanges index 8df0364e4c..21fa02ae9d 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -14,6 +14,10 @@ libavutil: 2021-04-27 API changes, most recent first: +2022-01-xx - xxxxxxxxxx - lavu fifo.h + Access to all AVFifoBuffer members is deprecated. The struct will + become an incomplete type in a future major libavutil version. + 2022-01-04 - 78dc21b123e - lavu 57.16.100 - frame.h Add AV_FRAME_DATA_DOVI_METADATA. diff --git a/libavutil/fifo.c b/libavutil/fifo.c index f2f046b1f3..aaade01333 100644 --- a/libavutil/fifo.c +++ b/libavutil/fifo.c @@ -28,9 +28,24 @@ #define FIFO_SIZE_MAX FFMIN3((uint64_t)INT_MAX, (uint64_t)UINT32_MAX, (uint64_t)SIZE_MAX) +#if FF_API_FIFO_PUBLIC +# define CTX_STRUCT_NAME FifoBuffer +#else +# define CTX_STRUCT_NAME AVFifoBuffer +#endif + +typedef struct CTX_STRUCT_NAME { + // These fields must match then contents of AVFifoBuffer in fifo.h + // until FF_API_FIFO_PUBLIC is removed + uint8_t *buffer; + uint8_t *rptr, *wptr, *end; + uint32_t rndx, wndx; + ///////////////////////////////////////// +} FifoBuffer; + AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size) { - AVFifoBuffer *f; + FifoBuffer *f; void *buffer; if (nmemb > FIFO_SIZE_MAX / size) @@ -39,15 +54,15 @@ AVFifoBuffer *av_fifo_alloc_array(size_t nmemb, size_t size) buffer = av_realloc_array(NULL, nmemb, size); if (!buffer) return NULL; - f = av_mallocz(sizeof(AVFifoBuffer)); + f = av_mallocz(sizeof(*f)); if (!f) { av_free(buffer); return NULL; } f->buffer = buffer; f->end = f->buffer + nmemb * size; - av_fifo_reset(f); - return f; + av_fifo_reset((AVFifoBuffer*)f); + return (AVFifoBuffer*)f; } AVFifoBuffer *av_fifo_alloc(unsigned int size) diff --git a/libavutil/fifo.h b/libavutil/fifo.h index f4fd291e59..ca4e7fe060 100644 --- a/libavutil/fifo.h +++ b/libavutil/fifo.h @@ -28,11 +28,21 @@ #include "avutil.h" #include "attributes.h" -typedef struct AVFifoBuffer { +#if FF_API_FIFO_PUBLIC +/** + * The contents of the struct are private and should not be accessed by the + * callers in any way. + */ +#endif +typedef struct AVFifoBuffer +#if FF_API_FIFO_PUBLIC +{ uint8_t *buffer; uint8_t *rptr, *wptr, *end; uint32_t rndx, wndx; -} AVFifoBuffer; +} +#endif +AVFifoBuffer; /** * Initialize an AVFifoBuffer. diff --git a/libavutil/tests/fifo.c b/libavutil/tests/fifo.c index a17d913233..e5aa88d252 100644 --- a/libavutil/tests/fifo.c +++ b/libavutil/tests/fifo.c @@ -18,7 +18,7 @@ #include <stdio.h> #include <stdlib.h> -#include "libavutil/fifo.h" +#include "libavutil/fifo.c" int main(void) { diff --git a/libavutil/version.h b/libavutil/version.h index 953aac9d94..7c031f547e 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -110,6 +110,7 @@ #define FF_API_COLORSPACE_NAME (LIBAVUTIL_VERSION_MAJOR < 58) #define FF_API_AV_MALLOCZ_ARRAY (LIBAVUTIL_VERSION_MAJOR < 58) #define FF_API_FIFO_PEEK2 (LIBAVUTIL_VERSION_MAJOR < 58) +#define FF_API_FIFO_PUBLIC (LIBAVUTIL_VERSION_MAJOR < 58) /** * @}