Skip to content

lib: silence clang/gcc -Wvla warnings in brotli headers #10738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

lib: silence clang/gcc -Wvla warnings in brotli headers #10738

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Mar 10, 2023 •
edited
Loading

brotli v1.0.0 throughout current latest v1.0.9 and latest master 1 trigger this warning.

llvm/clang:

In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:204:34: warning: variable length array used [-Wvla]
    const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./brotli/x64-ucrt/usr/include/brotli/port.h:253:34: note: expanded from macro 'BROTLI_ARRAY_PARAM'
#define BROTLI_ARRAY_PARAM(name) (name)
                                 ^~~~~~
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:206:48: warning: variable length array used [-Wvla]
    uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]);
                           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
./brotli/x64-ucrt/usr/include/brotli/port.h:253:35: note: expanded from macro 'BROTLI_ARRAY_PARAM'
#define BROTLI_ARRAY_PARAM(name) (name)
                                 ~^~~~~

gcc:

In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:204:5: warning: ISO C90 forbids variable length array 'encoded_buffer' [-Wvla]
  204 |     const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
      |     ^~~~~
./brotli/x64-ucrt/usr/include/brotli/decode.h:206:5: warning: ISO C90 forbids variable length array 'decoded_buffer' [-Wvla]
  206 |     uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]);
      |     ^~~~~~~

Closes #10738

Footnotes

  1. https://github.com/google/brotli/commit/ed1995b6bda19244070ab5d331111f16f67c8054 ↩

@vszakats vszakats added the build label Mar 10, 2023
@vszakats vszakats force-pushed the brotli-silence-wvla branch from 5247cdd to 2d2bd88 Compare March 10, 2023 14:15
vszakats added a commit to curl/curl-for-win that referenced this pull request Mar 10, 2023
@vszakats
curl-gnumake.sh: move -Wvla silencing upstream [ci skip]
5dbe29f 
Upstream solution works for all build methods, not just GNU Make.

Ref: curl/curl#10738
@vszakats vszakats force-pushed the brotli-silence-wvla branch from 2d2bd88 to c0e7597 Compare March 10, 2023 14:39
@vszakats
lib: silence clang/gcc -Wvla warnings in brotli headers
bd7bc96 
brotli v1.0.0 throughout current latest v1.0.9 and latest master [1]
trigger this warning.

llvm/clang:
```
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:204:34: warning: variable length array used [-Wvla]
    const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./brotli/x64-ucrt/usr/include/brotli/port.h:253:34: note: expanded from macro 'BROTLI_ARRAY_PARAM'
                                 ^~~~~~
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:206:48: warning: variable length array used [-Wvla]
    uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]);
                           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
./brotli/x64-ucrt/usr/include/brotli/port.h:253:35: note: expanded from macro 'BROTLI_ARRAY_PARAM'
                                 ~^~~~~
```

gcc:
```
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:204:5: warning: ISO C90 forbids variable length array 'encoded_buffer' [-Wvla]
  204 |     const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
      |     ^~~~~
./brotli/x64-ucrt/usr/include/brotli/decode.h:206:5: warning: ISO C90 forbids variable length array 'decoded_buffer' [-Wvla]
  206 |     uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]);
      |     ^~~~~~~
```

[1] google/brotli@ed1995b

Closes #xxxxx
@vszakats vszakats force-pushed the brotli-silence-wvla branch from c0e7597 to bd7bc96 Compare March 10, 2023 14:40
bagder
bagder approved these changes Mar 10, 2023
View reviewed changes
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we generally avoid this warning in configure builds thanks to the convert -I options to -isystem macro.

@vszakats
Copy link
Member Author

Aha, that's why it never appeared there, neat. This can work with GNU Make, but not sure how teach it to CMake.

@MarcelRaad
Copy link
Member

not sure how teach it to CMake

By giving the SYSTEM option to include_directories. This is documented since at least CMake 3.0:
https://cmake.org/cmake/help/v3.0/command/include_directories.html

@vszakats
Copy link
Member Author

vszakats commented Mar 10, 2023 •
edited
Loading

@MarcelRaad: Thank you, noted the trick!

[ But.. I'd prefer to do this in the source. It's easier than syncing behaviour between build systems. Unless there is an objection! ]

@vszakats vszakats closed this in b725fe1 Mar 10, 2023
@vszakats vszakats deleted the brotli-silence-wvla branch March 10, 2023 22:27
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
@vszakats @bch
lib: silence clang/gcc -Wvla warnings in brotli headers
04ae36e 
brotli v1.0.0 throughout current latest v1.0.9 and latest master [1]
trigger this warning.

It happened with CMake and GNU Make. autotools builds avoid it with
the `convert -I options to -isystem` macro.

llvm/clang:
```
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:204:34: warning: variable length array used [-Wvla]
    const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./brotli/x64-ucrt/usr/include/brotli/port.h:253:34: note: expanded from macro 'BROTLI_ARRAY_PARAM'
                                 ^~~~~~
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:206:48: warning: variable length array used [-Wvla]
    uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]);
                           ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
./brotli/x64-ucrt/usr/include/brotli/port.h:253:35: note: expanded from macro 'BROTLI_ARRAY_PARAM'
                                 ~^~~~~
```

gcc:
```
In file included from ./curl/lib/content_encoding.c:36:
./brotli/x64-ucrt/usr/include/brotli/decode.h:204:5: warning: ISO C90 forbids variable length array 'encoded_buffer' [-Wvla]
  204 |     const uint8_t encoded_buffer[BROTLI_ARRAY_PARAM(encoded_size)],
      |     ^~~~~
./brotli/x64-ucrt/usr/include/brotli/decode.h:206:5: warning: ISO C90 forbids variable length array 'decoded_buffer' [-Wvla]
  206 |     uint8_t decoded_buffer[BROTLI_ARRAY_PARAM(*decoded_size)]);
      |     ^~~~~~~
```

[1] google/brotli@ed1995b

Reviewed-by: Daniel Stenberg
Reviewed-by: Marcel Raad
Closes curl#10738
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bagder bagder bagder approved these changes

@MarcelRaad MarcelRaad MarcelRaad approved these changes

Assignees
No one assigned
Labels
build
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vszakats @MarcelRaad @bagder