From 8bd272f45b2f42f279389cb0c5430863dd939db7 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Fri, 28 Mar 2025 10:02:20 +0100 Subject: [PATCH 1/5] Have AVIF_FALSE and AVIF_TRUE be enums --- include/avif/avif.h | 7 +++++-- src/read.c | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/include/avif/avif.h b/include/avif/avif.h index e43eb01ddf..fdc1585436 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -85,8 +85,11 @@ extern "C" { ((AVIF_VERSION_MAJOR * 1000000) + (AVIF_VERSION_MINOR * 10000) + (AVIF_VERSION_PATCH * 100) + AVIF_VERSION_DEVEL) typedef int avifBool; -#define AVIF_TRUE 1 -#define AVIF_FALSE 0 +enum avifTrueFalse +{ + AVIF_FALSE = 0, + AVIF_TRUE = 1 +}; #define AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE 256 diff --git a/src/read.c b/src/read.c index e930658ffe..384921dd8c 100644 --- a/src/read.c +++ b/src/read.c @@ -3984,7 +3984,7 @@ static avifResult avifParseMovieBox(avifDecoderData * data, static avifProperty * avifMetaCreateProperty(avifMeta * meta, const char * propertyType) { avifProperty * metaProperty = avifArrayPush(&meta->properties); - AVIF_CHECK(metaProperty); + AVIF_CHECKERR(metaProperty, NULL); memcpy(metaProperty->type, propertyType, 4); return metaProperty; } @@ -3992,7 +3992,7 @@ static avifProperty * avifMetaCreateProperty(avifMeta * meta, const char * prope static avifProperty * avifDecoderItemAddProperty(avifDecoderItem * item, const avifProperty * metaProperty) { avifProperty * itemProperty = avifArrayPush(&item->properties); - AVIF_CHECK(itemProperty); + AVIF_CHECKERR(itemProperty, NULL); *itemProperty = *metaProperty; return itemProperty; } From e9d5df0edb2d162028ae05b3e5658977cad9c590 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Mon, 31 Mar 2025 14:39:36 +0200 Subject: [PATCH 2/5] Use typedef for enum --- include/avif/avif.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/avif/avif.h b/include/avif/avif.h index fdc1585436..510d9fd75f 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -85,11 +85,11 @@ extern "C" { ((AVIF_VERSION_MAJOR * 1000000) + (AVIF_VERSION_MINOR * 10000) + (AVIF_VERSION_PATCH * 100) + AVIF_VERSION_DEVEL) typedef int avifBool; -enum avifTrueFalse +typedef enum avifTrueFalse { AVIF_FALSE = 0, AVIF_TRUE = 1 -}; +} avifTrueFalse; #define AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE 256 From eef44ef503cf71aa859d9d333ebc8c44a3dbbf4d Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Mon, 31 Mar 2025 14:48:09 +0200 Subject: [PATCH 3/5] Add AVIF_ASSERT_NOT_REACHED_OR_RETURN macro --- apps/avifdec.c | 2 +- include/avif/internal.h | 6 ++++++ src/read.c | 2 +- src/write.c | 4 ++-- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/apps/avifdec.c b/apps/avifdec.c index f30a90cb11..d937aebfe0 100644 --- a/apps/avifdec.c +++ b/apps/avifdec.c @@ -359,7 +359,7 @@ int main(int argc, char * argv[]) const avifBool decodeAllFrames = frameIndex == DECODE_ALL_FRAMES; int currIndex = decodeAllFrames ? 0 : frameIndex; - while (AVIF_TRUE) { + for (;;) { result = decodeAllFrames ? avifDecoderNextImage(decoder) : avifDecoderNthImage(decoder, frameIndex); if (result != AVIF_RESULT_OK) { break; diff --git a/include/avif/internal.h b/include/avif/internal.h index 4e93c438a9..c304181d02 100644 --- a/include/avif/internal.h +++ b/include/avif/internal.h @@ -62,8 +62,14 @@ static inline void avifBreakOnError() // AVIF_ASSERT_OR_RETURN() can be used instead of assert() for extra security in release builds. #ifdef NDEBUG #define AVIF_ASSERT_OR_RETURN(A) AVIF_CHECKERR((A), AVIF_RESULT_INTERNAL_ERROR) +#define AVIF_ASSERT_NOT_REACHED_OR_RETURN \ + do { \ + avifBreakOnError(); \ + return AVIF_RESULT_INTERNAL_ERROR; \ + } while (0) #else #define AVIF_ASSERT_OR_RETURN(A) assert(A) +#define AVIF_ASSERT_NOT_REACHED_OR_RETURN assert(AVIF_FALSE); #endif // --------------------------------------------------------------------------- diff --git a/src/read.c b/src/read.c index 384921dd8c..9510a58d92 100644 --- a/src/read.c +++ b/src/read.c @@ -5479,7 +5479,7 @@ static avifResult avifMetaFindAlphaItem(avifMeta * meta, for (uint32_t dimgIdx = 0; dimgIdx < tileCount; ++dimgIdx) { if (dimgIdxToAlphaItemIdx[dimgIdx] >= meta->items.count) { avifFree(dimgIdxToAlphaItemIdx); - AVIF_ASSERT_OR_RETURN(AVIF_FALSE); + AVIF_ASSERT_NOT_REACHED_OR_RETURN; } avifDecoderItem * alphaTileItem = meta->items.item[dimgIdxToAlphaItemIdx[dimgIdx]]; alphaTileItem->dimgForID = (*alphaItem)->id; diff --git a/src/write.c b/src/write.c index a4e5f012b9..9beeb2bf62 100644 --- a/src/write.c +++ b/src/write.c @@ -2678,7 +2678,7 @@ static avifResult avifEncoderWriteMiniBox(avifEncoder * encoder, avifRWStream * if (floatFlag) { // bit(2) bit_depth_log2_minus4; - AVIF_ASSERT_OR_RETURN(AVIF_FALSE); + AVIF_ASSERT_NOT_REACHED_OR_RETURN; } else { AVIF_CHECKRES(avifRWStreamWriteBits(s, image->depth > 8, 1)); // bit(1) high_bit_depth_flag; if (image->depth > 8) { @@ -2746,7 +2746,7 @@ static avifResult avifEncoderWriteMiniBox(avifEncoder * encoder, avifRWStream * AVIF_CHECKRES(avifRWStreamWriteBits(s, gainmapFloatFlag, 1)); // bit(1) gainmap_float_flag; if (gainmapFloatFlag) { // bit(2) gainmap_bit_depth_log2_minus4; - AVIF_ASSERT_OR_RETURN(AVIF_FALSE); + AVIF_ASSERT_NOT_REACHED_OR_RETURN; } else { AVIF_CHECKRES(avifRWStreamWriteBits(s, gainmap->depth > 8, 1)); // bit(1) gainmap_high_bit_depth_flag; if (gainmap->depth > 8) { From da6062e03280d28d7b0cba38a35affaf4b61c174 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Mon, 31 Mar 2025 21:42:01 +0200 Subject: [PATCH 4/5] Fixes according to comments and add CI --- .github/workflows/ci-c-warnings.yml | 16 ++++++++++++++-- include/avif/avif.h | 4 ++-- include/avif/internal.h | 2 +- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci-c-warnings.yml b/.github/workflows/ci-c-warnings.yml index 81380569ff..3edf448802 100644 --- a/.github/workflows/ci-c-warnings.yml +++ b/.github/workflows/ci-c-warnings.yml @@ -16,10 +16,10 @@ jobs: strategy: fail-fast: false # Generate the configurations: - # case 0: nodiscard + # case 0: nodiscard, case 1: use stdbool, case 2: use enums matrix: os: [ubuntu-latest, windows-latest] - case: [0] + case: [0, 1, 2] runs-on: ${{ matrix.os }} @@ -53,6 +53,18 @@ jobs: - name: Enable nodiscard if: ${{ matrix.case == 0}} run: echo "CMAKE_AVIF_FLAGS=\"-DAVIF_ENABLE_NODISCARD=ON\"" >> $GITHUB_ENV + - name: Use stdbool for AVIF_TRUE/AVIF_FALSE + if: ${{ matrix.case == 1}} + run: | + sed -i 's/#include /#include \n#include /' include/avif/avif.h + sed -i 's/typedef int avifBool;/typedef bool avifBool;/' include/avif/avif.h + sed -i 's/#define AVIF_TRUE 1/#define AVIF_TRUE true/' include/avif/avif.h + sed -i 's/#define AVIF_FALSE 0/#define AVIF_FALSE false/' include/avif/avif.h + - name: Use enums for AVIF_TRUE/AVIF_FALSE + if: ${{ matrix.case == 2}} + run: | + sed -i 's/#define AVIF_TRUE 1/enum avifTrueFalse_{ AVIF_TRUE 1,/' include/avif/avif.h + sed -i 's/#define AVIF_FALSE 0/AVIF_FALSE 0};/' include/avif/avif.h - name: Prepare libavif (cmake) run: > diff --git a/include/avif/avif.h b/include/avif/avif.h index 510d9fd75f..e2ef222ead 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -85,11 +85,11 @@ extern "C" { ((AVIF_VERSION_MAJOR * 1000000) + (AVIF_VERSION_MINOR * 10000) + (AVIF_VERSION_PATCH * 100) + AVIF_VERSION_DEVEL) typedef int avifBool; -typedef enum avifTrueFalse +enum avifTrueFalse_ { AVIF_FALSE = 0, AVIF_TRUE = 1 -} avifTrueFalse; +}; #define AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE 256 diff --git a/include/avif/internal.h b/include/avif/internal.h index c304181d02..1ffb9a4025 100644 --- a/include/avif/internal.h +++ b/include/avif/internal.h @@ -69,7 +69,7 @@ static inline void avifBreakOnError() } while (0) #else #define AVIF_ASSERT_OR_RETURN(A) assert(A) -#define AVIF_ASSERT_NOT_REACHED_OR_RETURN assert(AVIF_FALSE); +#define AVIF_ASSERT_NOT_REACHED_OR_RETURN assert(0); #endif // --------------------------------------------------------------------------- From 279e267fc6585892690624bb187435f803f1e595 Mon Sep 17 00:00:00 2001 From: Vincent Rabaud Date: Fri, 4 Apr 2025 11:14:56 +0200 Subject: [PATCH 5/5] Revert change to only happen in CI --- .github/workflows/ci-c-warnings.yml | 4 ++-- include/avif/avif.h | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci-c-warnings.yml b/.github/workflows/ci-c-warnings.yml index 3edf448802..47ef787de9 100644 --- a/.github/workflows/ci-c-warnings.yml +++ b/.github/workflows/ci-c-warnings.yml @@ -63,8 +63,8 @@ jobs: - name: Use enums for AVIF_TRUE/AVIF_FALSE if: ${{ matrix.case == 2}} run: | - sed -i 's/#define AVIF_TRUE 1/enum avifTrueFalse_{ AVIF_TRUE 1,/' include/avif/avif.h - sed -i 's/#define AVIF_FALSE 0/AVIF_FALSE 0};/' include/avif/avif.h + sed -i 's/#define AVIF_TRUE 1/enum avifTrueFalse_{ AVIF_TRUE = 1,/' include/avif/avif.h + sed -i 's/#define AVIF_FALSE 0/AVIF_FALSE = 0};/' include/avif/avif.h - name: Prepare libavif (cmake) run: > diff --git a/include/avif/avif.h b/include/avif/avif.h index e2ef222ead..e43eb01ddf 100644 --- a/include/avif/avif.h +++ b/include/avif/avif.h @@ -85,11 +85,8 @@ extern "C" { ((AVIF_VERSION_MAJOR * 1000000) + (AVIF_VERSION_MINOR * 10000) + (AVIF_VERSION_PATCH * 100) + AVIF_VERSION_DEVEL) typedef int avifBool; -enum avifTrueFalse_ -{ - AVIF_FALSE = 0, - AVIF_TRUE = 1 -}; +#define AVIF_TRUE 1 +#define AVIF_FALSE 0 #define AVIF_DIAGNOSTICS_ERROR_BUFFER_SIZE 256