Any chance std::fopen() call can be changed to std::fopen_s()?

[misuo]

In sendfile_defs_default.hpp the restinio open_file() function uses std::fopen().

Is there any chance this can be changed to using std::fopen_s()?

Should be a simple change:

inline file_descriptor_t
open_file( const char * file_path )
{
	file_descriptor_t file_descriptor;
        std::fopen_s( &file_descriptor, file_path, "rb" ); // return code ignored.
  ...
}

Background

I'm using MSVC and want to build with security enhancements in the CRT library, in which fopen_s() is prefered, see MS documentation.

When compiling my code with "treat warnings as errors" and using restinio, I see...

...\restinio\sendfile_defs_default.hpp(42,43): error C2220: the following warning is treated as an error
	file_descriptor_t file_descriptor = std::fopen( file_path, "rb" );
...\sendfile_defs_default.hpp(42,43): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. (compiling source file 

I do not want to define the _CRT_SECURE_NO_WARNINGS as this will disable security enhancements for the rest of the code. I think part of the problem is that open_file() is an inline function.

[eao197]

Hi!

I think it's possible to change RESTinio's code to do check of RESTINIO_USE_FOPEN_S preprocessor symbol. If such symbol is defined then RESTinio will use std::fopen_s instead of std::fopen.

In that case you have to add RESTINIO_USE_FOPEN_S to your project's predefined symbols.

[misuo]

BTW: It should probably not be std::fopen_s as this is not part of C++ (not defined in cstdio), so plain fopen_s without std namespace.

[ngrodzitski]

Why simply not to use fopen_s as the only way? Is there any case it is not available with compiler that already C++17 at minimum?

If fopen_s is always available then having RESTINIO_USE_FOPEN_S is redundant.

[eao197]

If I understand correctly, the presence of fopen_s isn't related to C++ standard. It's a part of C11 standard.

And MS complier is a strange thing: it has no appropriate support (AFAIK) for receint C standards, but provides a lot of functions with _s suffix and "secure alternatives".

[ngrodzitski]

https://en.cppreference.com/w/cpp/17

"Major portion of C11 standard library" => https://wg21.link/P0063R3 it does have "fopen_s" in it.

[eao197]

"Major portion of C11 standard library" => https://wg21.link/P0063R3 it does have "fopen_s" in it.

It's great. So we can just call fopen_s and wait while someone opens an issues with absence of fopen_s on his/her plaform :)
Speaking seriously, if we can just use fopen_s then it's a really good thing.

[eao197]

What I don't understand is how to check the changes and how @misuo got the error because on Windows platform sendfile_defs_default.hpp should not be included, because on Windows sendfile_defs_win.hpp has to be included:

restinio/dev/restinio/sendfile.hpp

Lines 30 to 31 in 2773e06

	#if defined( _MSC_VER ) || defined(__MINGW32__)
	#include "sendfile_defs_win.hpp"

[misuo]

What I don't understand is how to check the changes and how @misuo got the error because on Windows platform sendfile_defs_default.hpp should not be included, because on Windows sendfile_defs_win.hpp has to be included:

sendfile_defs_default.hpp is included at the bottom of sendfile_defs_win.hpp when RESTINIO_ASIO_HAS_WINDOWS_OVERLAPPED_PTR is not defined. I'm not familiar with what that define do.

[eao197]

@misuo do you use Boost.Asio?

[eao197]

The RESTINIO_ASIO_HAS_WINDOWS_OVERLAPPED_PTR is defined here (if standalone Asio is used):

restinio/dev/restinio/asio_include.hpp

Lines 54 to 57 in 1f95dbc

	#if defined(ASIO_HAS_WINDOWS_OVERLAPPED_PTR)
	// Define feature macro with the same name for stand-alone and boost asio.
	#define RESTINIO_ASIO_HAS_WINDOWS_OVERLAPPED_PTR
	#endif

or here (if Boost.Asion is used):

restinio/dev/restinio/asio_include.hpp

Lines 106 to 109 in 1f95dbc

	#if defined(BOOST_ASIO_HAS_WINDOWS_OVERLAPPED_PTR)
	// Define feature macro with the same name for stand-alone and boost asio.
	#define RESTINIO_ASIO_HAS_WINDOWS_OVERLAPPED_PTR
	#endif

If ASIO_HAS_WINDOWS_OVERLAPPED_PTR (or BOOST_ASIO_HAS_WINDOWS_OVERLAPPED_PTR) is not defined then Asio can't detect the presence of overlapped API. And it's a strange thing as for me. I don't understand how it can be. The corresponding fragment from Asio source code: https://github.com/chriskohlhoff/asio/blob/fa27820c05afd740fa2adc1ecfb9da5afe026443/asio/include/asio/detail/config.hpp#L934-L940
And this is how ASIO_HAS_IOCP is detected: https://github.com/chriskohlhoff/asio/blob/fa27820c05afd740fa2adc1ecfb9da5afe026443/asio/include/asio/detail/config.hpp#L790-L800
Maybe the ASIO_DISABLE_WINDOWS_OVERLAPPED_PTR is defined somehow?

[misuo]

@misuo do you use Boost.Asio?

No, only used as part of restinio. I do have the boost library in path and the only defines I have set for restinio is RESTINIO_FMT_HEADER_ONLY and FMT_HEADER_ONLY.

[eao197]

Thanks for clarification, @misuo

Anyway, it's a very strange situation because in the normal circumstances the sendfile_defs_default.hpp should not be included and you should not get errors about using std::fopen. Inclusion of the sendfile_defs_default.hpp also means that Win32-specific implementation of sendfile won't be used (and the default one isn't as efficient as Win32-specific one).