Skip to content

Fix some miscellaneous MSVC warnings #7

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix some miscellaneous MSVC warnings #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b02959
Fixed some MSVC warnings
RandomEden Jan 20, 2025
d5c1804
Merge branch 'PerformanC:main' into fix_msvc_warnings
0xfadead Jan 20, 2025
bc7c20a
Change `_really_` to `REALLY` in comment
0xfadead Feb 4, 2025
e29a3d7
Added extra line for formatting
0xfadead Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion cthreads.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,11 @@ void cthreads_thread_exit(void *code) {
#endif

#ifdef _WIN32
#if defined __WATCOMC__ || _MSC_VER || __DMC__
/* NOTE: This gives warnings on MSVC, so I removed this
* for the oldest version of it I could test (19.20). */
/* TODO: Test with the other described compilers if this
* is REALLY necessary. */
#if defined __WATCOMC__ || _MSC_VER < 1920 || __DMC__
ExitThread((DWORD)code);
#else
ExitThread((DWORD)(uintptr_t)code);
Expand Down Expand Up @@ -323,6 +327,8 @@ int cthreads_cond_destroy(struct cthreads_cond *cond) {
#endif

#ifdef _WIN32
(void) cond;

return 0;
#else
return pthread_cond_destroy(&cond->pCond);
Expand Down
2 changes: 2 additions & 0 deletions cthreads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ struct cthreads_args {
};

#ifdef _WIN32
/* MSVC sees strncpy() as insecure. */
#define _CRT_SECURE_NO_WARNINGS
Comment on lines +10 to +11
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/* MSVC sees strncpy() as insecure. */
#define _CRT_SECURE_NO_WARNINGS
/* INFO: MSVC sees strncpy() as insecure. */
#define _CRT_SECURE_NO_WARNINGS

Copy link
Member

Choose a reason for hiding this comment

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

Also, is this widely compatible or affects files outside the project? For second case, I believe we can #undef it? Wouldn't be good to apply such for files outside.

Copy link
Contributor Author

@0xfadead 0xfadead Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm afraid there's no easy way to do this.
We could of course undef it, but it would still propagate because of windows.h's include guards.
The best solution would be to not require windows.h in the header, but the only way to do this is making all structs (or parts of structs using specific types) opaque.

I think the best course of action is just to remove the define and endur the warning or add an option for the user to disable the define.

Copy link
Member

@ThePedroo ThePedroo Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

In my opinion we can should* strncpy for memcpy and just handle NULL terminator ending. With proper testing, it becomes safe anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I can find out in which version strncpy_s() was introduced, I could probably make it version dependent.

#include <windows.h>
#define CTHREADS_SEMAPHORE 1
#else
Expand Down