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

C2X - Deprecated Code Style #2983

Open
dylanetaft opened this issue Oct 18, 2023 · 5 comments
Open

C2X - Deprecated Code Style #2983

dylanetaft opened this issue Oct 18, 2023 · 5 comments

Comments

@dylanetaft
Copy link
Contributor

I am compiling for Windows in MSYS2, both MINGW64 and CLANG64.

I was sort of looking at it, I think the compiler is upset about

#define __P(x) x
static char * _progname __P((char *));

static char *
_progname(nargv0)
char * nargv0;
{}

and a few other functions that do not have normal prototypes before their definition.

There's many ports of getopt for windows.
This would probably work if I use compiler flags to not compile against version C2X, which is the next standard, but the coding style seems to be deprecated.

Should some other port of getopt be brought in? This code is from 1994 it looks. It seems like it's probably going to break for others soon?

-apps\CMakeFiles\test-server.dir__\win32port\win32helpers\getopt.c.obj.d -o extern_libs/libwebsockets/test-apps/CMakeFiles/test-server.dir/__/win32port/win32helpers/getopt.c.obj -c D:/Projects/15Game/wallet/extern_libs/libwebsockets/win32port/win32helpers/getopt.c
D:/Projects/15Game/wallet/extern_libs/libwebsockets/win32port/win32helpers/getopt.c:63:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
_progname(nargv0)
^
D:/Projects/15Game/wallet/extern_libs/libwebsockets/win32port/win32helpers/getopt.c:63:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
D:/Projects/15Game/wallet/extern_libs/libwebsockets/win32port/win32helpers/getopt.c:87:1: error: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Werror,-Wdeprecated-non-prototype]
getopt(nargc, nargv, ostr)
^
3 errors generated.
[206/220] Building C object extern_libs/libwebso...pps/CMakeFiles/test-server.dir/test-server.c.obj
ninja: build stopped: subcommand failed.

@dylanetaft
Copy link
Contributor Author

dylanetaft commented Oct 18, 2023

Workaround:
In CMakeLiles.txt including LWS as a module
.....
.....
set(C_STANDARD 17)
set(C_STANDARD_REQUIRED ON)
OPTION(DISABLE_WERROR "" ON)
add_subdirectory(extern_libs/libwebsockets)
....
....
Will allow LWS to compile with the deprecated code in
win32helpers/getopt.c
win32helpers/getopt_long.c

@lws-team
Copy link
Member

set(C_STANDARD 17)

This isn't required to build lws standalone with windows last time I did it... presumably you are overriding this directly or indirectly in your outer cmake project and it's inheriting that.

@dylanetaft
Copy link
Contributor Author

dylanetaft commented Oct 18, 2023

Yeah, I did add it in the outer cmake, because it was complaining about C2X not supporting what's in those two .c files.
I was doing some testing...I think getopt and getopt_long are actually part of ucrt C lib now....don't need helpers.

While this temporary workaround is fine...would a permanent fix be to add CHECK_C_SOURCE_COMPILES tests in CMakeLists.txt in /lib/plat/windows to test for functioning getopt and getopt_long without the helpers?

And then split them into their own helper subdirectory and do a find-replace everywhere ${WIN32_HELPERS_PATH} is mentioned?

Or is this a non-issue - kick the can a year and when C23 is a standard and default in environments...set the C version to a lower release and disable warnings? It's only an issue because warnings default to error in the default settings for LWS CMake.

Disabling WERROR is really what makes it work. It's the only warning that is occuring too, rest of build is fine.

I don't know if it's worth spending any legitimate time on this until it's absolutely needed, as these functions are used in a bunch of sample examples, so a bunch of stuff needs to be touched for a fix....and I don't see too many people complaining. Maybe Visual Studio doesn't care about this deprecation and most people are using that, I'm the odd one out using MSYS2 + CLANG.

I could try to do the find\replace, but I don't have all your regression tests and whatnot that you do, it's something that could break param parsing on windows in examples if not well tested....

@lws-team
Copy link
Member

Testing if it exists without the helpers and only build the helpers is fine if modern toolchain on windows includes them already. The helpers are only there because when I did it, windows toolchain lacked them.

IIUI that shouldn't break the examples since it already exists in the link then, or is no different from today.

@dylanetaft
Copy link
Contributor Author

I am planning on getting to this within a few weeks and probably submitting a pr

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

No branches or pull requests

2 participants