[xrCore] Several Level 3 warnings eliminated

[antoncxx]

I have addressed several Level 3 warnings within the xrCore component of the Engine.

It's important to note that there are still multiple warnings remaining unresolved. These may necessitate further discussion as I have focused on rectifying issues that were straightforward and did not demand significant code alterations or excessive use of explicit static_cast, except in instances where, in my opinion, a static cast was necessary.

I believe this PR could be linked with #116.

[Xottab-DUTY]

#116 is a subtask of #713, so this is related to #713 :)

[antoncxx]

@Xottab-DUTY ,
Thank you for your comment.

I've addressed the spacing issue. However, I'm currently unable to pinpoint why the OpenBSD Debug build is failing, as I lack the means to establish a suitable testing environment.

Would you happen to have any insights or suggestions on this matter?

[Xottab-DUTY]

I've addressed the spacing issue. However, I'm currently unable to pinpoint why the OpenBSD Debug build is failing, as I lack the means to establish a suitable testing environment.

Would you happen to have any insights or suggestions on this matter?

Just ignore it for now. It's always failing.

[Xottab-DUTY]

What warning did you tried to fix with introducing xr_scanf?

[antoncxx]

What warning did you tried to fix with introducing xr_scanf?

When building on windows, the following warning occurs:

They suggest either using sscanf_s or disable this kind of warnings at all.

There is problem with 1st approach, because that function is only available on Windows. Redefining:
#define sscanf_s sscanf will not help, because functions have different interfaces: Confusion with sscanf_s

The second option would be defining #define _CRT_SECURE_NO_WARNINGS, this will disable all MSVC security warnings. I would also like to list functions, that MSVC considers unsafe, because there are bunch of them throughout the code:

String Manipulation Functions

strcpy -> strcpy_s
strcat -> strcat_s
sprintf -> sprintf_s
vsprintf -> vsprintf_s
scanf-> scanf_s
sscanf -> sscanf_s
gets -> gets_s (though gets is removed in C11 due to its inherent unsafety)

Memory Manipulation Functions

memcpy -> memcpy_s
memmove -> memmove_s
strncpy -> strncpy_s
strncat -> strncat_s

File Operation Functions

fopen -> fopen_s
freopen -> freopen_s
tmpnam -> tmpnam_s
sprintf -> sprintf_s
snprintf -> _snprintf_s or snprintf_s (depending on the version)

Other Functions

ctime -> ctime_s
asctime -> asctime_s
gmtime -> gmtime_s
localtime -> localtime_s
strtok -> strtok_s
makepath -> _makepath_s
splitpath -> _splitpath_s
getenv -> _dupenv_s or getenv_s (depending on the function's signature and usage)

Determining the optimal approach is essential, as redefining every 'unsafe' function could be time-consuming and might not fully address potential security issues. Alternatively, addressing these concerns could necessitate code refactoring and the adoption of modern C++ practices, which, while potentially involving more effort, directly addresses the underlying security concerns.

[Xottab-DUTY]

What warning did you tried to fix with introducing xr_scanf?

When building on windows, the following warning occurs:

They suggest either using sscanf_s or disable this kind of warnings at all.

There is problem with 1st approach, because that function is only available on Windows. Redefining: #define sscanf_s sscanf will not help, because functions have different interfaces: Confusion with sscanf_s

The second option would be defining #define _CRT_SECURE_NO_WARNINGS, this will disable all MSVC security warnings. I would also like to list functions, that MSVC considers unsafe, because there are bunch of them throughout the code:

String Manipulation Functions

strcpy -> strcpy_s strcat -> strcat_s sprintf -> sprintf_s vsprintf -> vsprintf_s scanf-> scanf_s sscanf -> sscanf_s gets -> gets_s (though gets is removed in C11 due to its inherent unsafety)

Memory Manipulation Functions

memcpy -> memcpy_s memmove -> memmove_s strncpy -> strncpy_s strncat -> strncat_s

File Operation Functions

fopen -> fopen_s freopen -> freopen_s tmpnam -> tmpnam_s sprintf -> sprintf_s snprintf -> _snprintf_s or snprintf_s (depending on the version)

Other Functions

ctime -> ctime_s asctime -> asctime_s gmtime -> gmtime_s localtime -> localtime_s strtok -> strtok_s makepath -> _makepath_s splitpath -> _splitpath_s getenv -> _dupenv_s or getenv_s (depending on the function's signature and usage)

Determining the optimal approach is essential, as redefining every 'unsafe' function could be time-consuming and might not fully address potential security issues. Alternatively, addressing these concerns could necessitate code refactoring and the adoption of modern C++ practices, which, while potentially involving more effort, directly addresses the underlying security concerns.

That's the problem I though about. It's better to just disable the warnings with _CRT_SECURE_NO_WARNINGS.
Historically, scanf_s was used throughout the engine, but it was replaced with scanf to make it work on Linux. Replacing scanf back to some sort of scanf_s is... a waste of resources.

Alternatively, addressing these concerns could necessitate code refactoring and the adoption of modern C++ practices, which, while potentially involving more effort, directly addresses the underlying security concerns.

That's the best option. Slowly rewrite some unsafe C-style code to C++17.
Though, in some cases, we can't touch the code at all to preserve compatibility with the original engine (and assets made for it).

[antoncxx]

@Xottab-DUTY,

I kindly ask you to review the current changes.

These changes do not completely fix all warnings within the xrCore module; there are several of them still left, but they might require more changes and are beyond the scope of this PR.

Thank you in advance.

[Xottab-DUTY]

@Xottab-DUTY,

I kindly ask you to review the current changes.

These changes do not completely fix all warnings within the xrCore module; there are several of them still left, but they might require more changes and are beyond the scope of this PR.

Thank you in advance.

Sorry for the long-time review! I was really busy with real life events during March.
Now I can review PRs freely.

Could you remove my commits (rebased by you) from this PR?

[antoncxx]

@Xottab-DUTY,
Thank you! Rebased the branch, please verify.