Skip to content
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.

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

test: remove CMAKE_C_STANDARD #714

Merged
merged 4 commits into from
Feb 27, 2023
Merged

test: remove CMAKE_C_STANDARD #714

merged 4 commits into from
Feb 27, 2023

Conversation

alfredh
Copy link
Contributor

@alfredh alfredh commented Feb 27, 2023

the current C99 action test is broken

The code is always compiled with C11.

@alfredh
Copy link
Contributor Author

alfredh commented Feb 27, 2023

Example:

alfredh@debian:~/git/re$ cmake -DCMAKE_C_STANDARD=99 .
[  0%] Building C object CMakeFiles/re-objs.dir/src/av1/depack.c.o
/usr/bin/cc -DARCH=\"x86_64\" -DHAVE_ACCEPT4 -DHAVE_ATOMIC -DHAVE_EPOLL -DHAVE_EXECINFO -DHAVE_FORK -DHAVE_GETIFADDRS -DHAVE_GETOPT -DHAVE_INET6 -DHAVE_PRCTL -DHAVE_PTHREAD -DHAVE_PWD_H -DHAVE_RESOLV -DHAVE_ROUTE_LIST -DHAVE_SELECT -DHAVE_SELECT_H -DHAVE_SETRLIMIT -DHAVE_SIGNAL -DHAVE_STRERROR_R -DHAVE_STRINGS_H -DHAVE_SYSLOG -DHAVE_SYS_TIME_H -DHAVE_UNAME -DHAVE_UNISTD_H -DHAVE_UNIXSOCK=1 -DLINUX -DOS=\"Linux\" -DUSE_DTLS -DUSE_OPENSSL -DUSE_OPENSSL_AES -DUSE_OPENSSL_DTLS -DUSE_OPENSSL_HMAC -DUSE_OPENSSL_SRTP -DUSE_TLS -DUSE_ZLIB -DVERSION=\"2.12.0\" -DVER_MAJOR=2 -DVER_MINOR=12 -DVER_PATCH=0 -D_GNU_SOURCE -I/home/alfredh/git/re/. -I/home/alfredh/git/re/include -fPIC -pedantic -Wall -Wbad-function-cast -Wcast-align -Wextra -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wno-strict-aliasing -Wold-style-definition -Wshadow -Waggregate-return -Wstrict-prototypes -Wuninitialized -Wvla -std=c11 -o CMakeFiles/re-objs.dir/src/av1/depack.c.o -c /home/alfredh/git/re/src/av1/depack.c

-> The compiler is still using "-std=c11"

it looks like CMAKE_C_STANDARD is hardcoded in the CMake file.

When building with a C99 compiler the test try_into fails due to missing definitions.

@alfredh alfredh changed the title Update CMakeLists.txt test: remove CMAKE_C_STANDARD Feb 27, 2023
@sreimers
Copy link
Member

I think both have to be removed, since test is loaded with subdirectory:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1cf5ec4..e340e10 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -53,7 +53,6 @@ option(USE_RTMP "Enable RTMP" ON)
 option(USE_SIP "Enable SIP" ON)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
-set(CMAKE_C_STANDARD 11)
 set(CMAKE_C_EXTENSIONS OFF)
 
 if(MSVC)
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index bd5a213..790ab1f 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -30,7 +30,6 @@ option(USE_SANITIZER "Sanitizers like: address, thread, undefined, memory")
 include(sanitizer)
 
 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
-set(CMAKE_C_STANDARD 11)
 set(CMAKE_C_EXTENSIONS OFF)
 set(CMAKE_BUILD_TYPE Debug)

@sreimers
Copy link
Member

This fixes try_into tests: #717

@alfredh
Copy link
Contributor Author

alfredh commented Feb 27, 2023

I guess the compiler will use the default standard, if not specified.

With this change, the C99 CI check should work.

@alfredh
Copy link
Contributor Author

alfredh commented Feb 27, 2023

build failed, as expected:

D:\a\re\re\src\tls\openssl\tls_udp.c(757): error C2220: the following warning is treated as an error
D:\a\re\re\src\tls\openssl\tls_udp.c(757): warning C4013: 'content_type_str' undefined; assuming extern returning int

lets merge the "C99 dbg" first ...

@alfredh
Copy link
Contributor Author

alfredh commented Feb 27, 2023

this should be working now ...

@sreimers sreimers merged commit 8f416f4 into main Feb 27, 2023
@sreimers sreimers deleted the alfredh-patch-2 branch February 27, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants