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

Issue concerning Net/CMakeLists.txt detection of HAVE_SENDFILE #4852

Open
lbakman opened this issue Jan 16, 2025 · 0 comments
Open

Issue concerning Net/CMakeLists.txt detection of HAVE_SENDFILE #4852

lbakman opened this issue Jan 16, 2025 · 0 comments
Labels

Comments

@lbakman
Copy link
Contributor

lbakman commented Jan 16, 2025

Background
I was investigating an issue with the conan recipe for Poco 1.13.3 while using MinGW. I modified the recipe for Poco 1.14.0 (locally) and stumbled upon a possible issue with 1.14.0.

Generally there seem to be a problem with discovering the correct value of HAVE_SENDFILE.

Even though I noticed this while building with MinGW, it does not actually affect the output using MinGW, because of:

if (MINGW)
	target_link_libraries(Net PUBLIC "mswsock.lib")
endif()

I will get back to that later.

Describe the bug
When building Poco 1.14.0 with MinGW 12.2.0 and CMake 3.16.3 I noticed the following oddity:

-- Looking for include file sys/sendfile.h
-- Looking for include file sys/sendfile.h - not found
-- Looking for sendfile
-- Looking for sendfile - not found
-- OS has native sendfile function
-- CMake 3.16.3 successfully configured Poco using MinGW Makefiles generator

Net/CMakeLists.txt does not find neither 'sys/sendfile.h' nor a 'sendfile' symbol, but still prints out a STATUS message that 'OS has native sendfile support'.

There seem to be a number of problems with the following block:

if (MSVC)
	set(HAVE_SENDFILE ON)
else()
	include(CheckIncludeFiles)
	include(CheckSymbolExists)
	check_include_files(sys/sendfile.h HAVE_SYS_SENDFILE_H)
	if(HAVE_SYS_SENDFILE_H)
		check_symbol_exists(sendfile sys/sendfile.h HAVE_SENDFILE)
		if (NOT DEFINED HAVE_SENDFILE)
			check_symbol_exists(sendfile64 sys/sendfile.h HAVE_SENDFILE)
		endif()
	else()
		# BSD version
		check_symbol_exists(sendfile "sys/types.h;sys/socket.h;sys/uio.h" HAVE_SENDFILE)
	endif()
endif()

if (DEFINED HAVE_SENDFILE)
	message(STATUS "OS has native sendfile function")
	add_compile_definitions(POCO_HAVE_SENDFILE)
endif()

My initial issue stems from if (DEFINED HAVE_SENDFILE). check_symbol_existswill define the variable HAVE_SENDFILE but set it to false.

This means that OS has native send function will always be written to the screen and POCO_HAVE_SENDFILE will always be defined.

The same problem exists for the line if (NOT DEFINED HAVE_SENDFILE). I do not believe the above block will ever check for sendfile64 as HAVE_SENDFILE is always defined but possibly false.

Even with the two above issues fixed, there seems to be a problem with check_symbol_exists(sendfile64 sys/sendfile.h HAVE_SENDFILE).

Because HAVE_SENDFILE is already defined, the check does not occur at all. Not sure if this is a bug in CMake 3.16.3 or expected behaviour. But, either HAVE_SENDFILE needs to be undefined again, using unset(HAVE_SENDFILE CACHE), or we need to use a different variable name, eg HAVE_SENDFILE64.

From my testing, with both Linux gcc 12 and MinGW 12, I believe the block should be:

if (MSVC OR MINGW)
	set(HAVE_SENDFILE ON)
else()
	include(CheckIncludeFiles)
	include(CheckSymbolExists)
	check_include_files(sys/sendfile.h HAVE_SYS_SENDFILE_H)
	if(HAVE_SYS_SENDFILE_H)
		check_symbol_exists(sendfile sys/sendfile.h HAVE_SENDFILE)
		if (NOT HAVE_SENDFILE)
			check_symbol_exists(sendfile64 sys/sendfile.h HAVE_SENDFILE64)
		endif()
	else()
		# BSD version
		check_symbol_exists(sendfile "sys/types.h;sys/socket.h;sys/uio.h" HAVE_SENDFILE)
	endif()
endif()

if (HAVE_SENDFILE OR HAVE_SENDFILE64)
	message(STATUS "OS has native sendfile function")
	add_compile_definitions(POCO_HAVE_SENDFILE)
endif()

The reason I am adding OR MINGW to the initial line is the addition of mswsock.lib later in the CMakeLists.txt as described in the beginning.

This causes SocketImpl::sendFile to link to TransmitFile which matches the ìfdef POCO_OS_FAMILY_WINDOWS` block. (This is the issue I am investigating in the conan recipe, as it does not propagate this dependency to the recipe consumer).

To Reproduce
Made different changes for test purposes:

  • Changed sys/sendfile.h to sys/sendfilex.h and re-ran configure
  • Changed sendfile to sendfilex and re-ran configure
  • Changed sendfile64 to sendfile64x and re-ran configure

Expected behavior
POCO_HAVE_SENDFILE should not be defined as a compile definition if sendfile is unavailable.

Please add relevant environment information:

  • Linux GCC 12 and Windows MinGW (12.2.0)
  • Poco version 1.14.0
  • CMake version 3.16.3
@lbakman lbakman added the bug label Jan 16, 2025
@matejk matejk added this to the Release 1.15.0 milestone Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants