fs: remove `_POSIX_C_SOURCE` defining

fanquake commented at 3:01 pm on May 9, 2025: member

On Linux systems, _POSIX_C_SOURCE will default to 200809L (since glibc 2.10). There’s currently no reason for us to undefine it, and then set it to an earlier value. Also tested with musl libc.

I think if anything, the project should be settings macros like _POSIX_C_SOURCE, globally.

DrahtBot commented at 3:02 pm on May 9, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32460.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto
Stale ACK	laanwj

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

laanwj commented at 3:59 pm on May 9, 2025: member

From what I can tell, compilers on Linux systems, will be defining _GNU_SOURCE, which results in glibc defining _POSIX_C_SOURCE to 200809L; so undefining it, and setting it to an earlier value does not seem like the correct behaviour for us, or even required

Yes, there is never a valid reason to set it to a lower value. The idea of the macro is to tell the header files “i know about POSIX version X and want to use it”.

~~Usually this is defined by the build system, or at most at the top of specific implementation files, defining them in a header that’s included by other stuff is really strange.~~ Edit: oh it’s defined in a cpp, that’s fine really. But unnecessary apparently otherwise this wouldn’t compile.

laanwj added the label Utils/log/libs on May 9, 2025

luke-jr commented at 11:54 am on May 14, 2025: member

From what I can tell, compilers on Linux systems, will be defining _GNU_SOURCE

_GNU_SOURCE isn’t supposed to ever be defined by default, and I don’t see us defining it anywhere…?

laanwj commented at 7:59 am on May 15, 2025: member

_GNU_SOURCE isn’t supposed to ever be defined by default, and I don’t see us defining it anywhere…?

Not sure when it became that way, but it seems to be set by default, just compile anything using g++ (first thought was it related to C++ standard, but it isn’t):

0#include <iostream>
1
2int main()
3{
4    std::cout << _GNU_SOURCE << std::endl;
5    std::cout << _POSIX_C_SOURCE << std::endl;
6    return 0;
7}

0$ g++ --version
1g++ (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0
2$ g++ test.cpp  -o test
3$ ./test
41
5200809

One thing i’m not sure about is if this is true on all the BSDs.

Then again, the removed code is explicitly __linux__ so that doesn’t matter for this PR.

hebasto commented at 2:11 pm on May 16, 2025: member

From what I can tell, compilers on Linux systems, will be defining _GNU_SOURCE

_GNU_SOURCE isn’t supposed to ever be defined by default, and I don’t see us defining it anywhere…?

Do we use GNU extensions?

laanwj commented at 12:34 pm on May 19, 2025: member

Do we use GNU extensions?

No, we don’t.

laanwj approved

laanwj commented at 12:35 pm on May 19, 2025: member

ACK 0574aeaf6608dc3e118a4d2c0613dab2097cf62e

hebasto commented at 2:45 pm on May 19, 2025: member

From what I can tell, compilers on Linux systems, will be defining _GNU_SOURCE…

Is this behaviour documented? If not, why rely on incidental behaviour that might change?

… which results in glibc defining _POSIX_C_SOURCE to 200809L…

I don’t see any causality here: passing -U_GNU_SOURCE to an arbitrary compiler on my machine still results in _POSIX_C_SOURCE being set to 200809L.

laanwj commented at 8:37 am on May 20, 2025: member

i think _GNU_SOURCE is a bit of a red herring here. The point here is that modern versions of g++/glibc on Linux set _POSIX_SOURCE high enough by default to be able to use posix_fallocate (a POSIX function, not a GNU extension function) out of the box, so there’s no more reason to set it explicitly.

i couldn’t find this behavior explicitly documented anywhere, but i’m sure a lot of software relies on that by now.

hebasto commented at 8:46 am on May 20, 2025: member

i think _GNU_SOURCE is a bit of a red herring here. The point here is that modern versions of g++/glibc on Linux set _POSIX_SOURCE high enough by default to be able to use posix_fallocate out of the box, so there’s no more reason to set it explicitly.

Then maybe remove _GNU_SOURCE mentioning from the PR description?

fs: remove _POSIX_C_SOURCE defining

On Linux systems, `_POSIX_C_SOURCE` will default to `200809L` (since
glibc 2.10). There's currently no reason for us to undefine it, and then
set it to an earlier value. Also tested with musl libc.

I think if anything, the project should be settings macros like
`_POSIX_C_SOURCE`, globally.

db895c0e38

fanquake force-pushed on May 20, 2025

hebasto approved

hebasto commented at 1:24 pm on May 20, 2025: member

ACK db895c0e38427ce73167ef325f5731a3c4805598.

DrahtBot requested review from laanwj on May 20, 2025

maflcko commented at 1:50 pm on May 20, 2025: member

I wonder if there is a platform without posix_fallocate (outside of macos and windows). If not, the fallback can probably be fully removed, along with the detection and it can just be assumed to be always present?

fanquake commented at 2:56 pm on May 20, 2025: member

 0181155a38912fab7e25d751d1f0281d7127ca8c309e2835bf0b109f30d425f91  guix-build-db895c0e3842/output/aarch64-linux-gnu/SHA256SUMS.part
 11b89a9dc265956572226b7a1558f8cb24c6f65f490c82382cbfba16500c91d9c  guix-build-db895c0e3842/output/aarch64-linux-gnu/bitcoin-db895c0e3842-aarch64-linux-gnu-debug.tar.gz
 27180be8208cfad61ba6fda74d10f942769a8e3c7cbd6a141b3d53e9905d1a76f  guix-build-db895c0e3842/output/aarch64-linux-gnu/bitcoin-db895c0e3842-aarch64-linux-gnu.tar.gz
 3459deb9325f13dc780a138e52e10f0a74b25f5dd605add338c451edf30af7715  guix-build-db895c0e3842/output/arm-linux-gnueabihf/SHA256SUMS.part
 4b7b3de67bce3cb28c5b2a68b37988eaf8972fecb2d74bd07c8c1205d4bbaed38  guix-build-db895c0e3842/output/arm-linux-gnueabihf/bitcoin-db895c0e3842-arm-linux-gnueabihf-debug.tar.gz
 5e2baf994c748e1a24450c4adb80862dbed82a89d2877c14657ff0642ac7c8014  guix-build-db895c0e3842/output/arm-linux-gnueabihf/bitcoin-db895c0e3842-arm-linux-gnueabihf.tar.gz
 67839b10062f0b19b1796655ac80579f2a8906b5818c986bee9f0f28cc899523c  guix-build-db895c0e3842/output/arm64-apple-darwin/SHA256SUMS.part
 75cef41bd838873b0cce491988bcab6538128d068406c454c06dced85d7f4e2a4  guix-build-db895c0e3842/output/arm64-apple-darwin/bitcoin-db895c0e3842-arm64-apple-darwin-codesigning.tar.gz
 8ef7c4cbe008ca853f52bf0ab72128bd8eef6f2d4d6cae7f453d7b3ca11d573c3  guix-build-db895c0e3842/output/arm64-apple-darwin/bitcoin-db895c0e3842-arm64-apple-darwin-unsigned.tar.gz
 93dcaa489a49b5dda2da4f70ca0fc0578a41c7e24b9026ed4207918193dca8436  guix-build-db895c0e3842/output/arm64-apple-darwin/bitcoin-db895c0e3842-arm64-apple-darwin-unsigned.zip
103671cb5b704ffc507c9024b3f527912f7e9219f4abd337095cb4e4a0baf8976a  guix-build-db895c0e3842/output/dist-archive/bitcoin-db895c0e3842.tar.gz
11174cd1ac3db45158bb343102d99ce30deffc37756b6c48796c1c4f9ead61e63d  guix-build-db895c0e3842/output/powerpc64-linux-gnu/SHA256SUMS.part
12d04326d8ceb13e5d95b8d29215c561f17386d980630b006577356ed70762507d  guix-build-db895c0e3842/output/powerpc64-linux-gnu/bitcoin-db895c0e3842-powerpc64-linux-gnu-debug.tar.gz
139ffbb172b614fbbd85fdc24259accb4a4d1aff17d53dbf26b78f58f757435822  guix-build-db895c0e3842/output/powerpc64-linux-gnu/bitcoin-db895c0e3842-powerpc64-linux-gnu.tar.gz
14d6e49fde14b75441438debff249abf4671426ad50a294d35e9aba1f1ed8ceb4f  guix-build-db895c0e3842/output/riscv64-linux-gnu/SHA256SUMS.part
15ca135608c2f22ddac41628eae67f9b1d26900bcf7b2a174009b448232eb5abee  guix-build-db895c0e3842/output/riscv64-linux-gnu/bitcoin-db895c0e3842-riscv64-linux-gnu-debug.tar.gz
161fc2be48c765cfdc7fee7e5797a8f79d727fafd8fe94263f4afb1bd75de69802  guix-build-db895c0e3842/output/riscv64-linux-gnu/bitcoin-db895c0e3842-riscv64-linux-gnu.tar.gz
17017cdb481967dab863d372e65dd653e1461476b30ee297b51898b568953d079b  guix-build-db895c0e3842/output/x86_64-apple-darwin/SHA256SUMS.part
18c82480ada743741d9bb657771106b4343fe473c822303df8707e985c9b19e5e8  guix-build-db895c0e3842/output/x86_64-apple-darwin/bitcoin-db895c0e3842-x86_64-apple-darwin-codesigning.tar.gz
19ea9863ef26db0dfcada4c9721d0a9ad9e1bf0971c95ebf911776261906a1f8fd  guix-build-db895c0e3842/output/x86_64-apple-darwin/bitcoin-db895c0e3842-x86_64-apple-darwin-unsigned.tar.gz
2003857667e9819c765f0d872f61f34aa5a80ab70b0725751257d1a365c3131855  guix-build-db895c0e3842/output/x86_64-apple-darwin/bitcoin-db895c0e3842-x86_64-apple-darwin-unsigned.zip
21921f171f605f9f2bd0c9dcdde3711cccf6fde0b4b82568f43a90df44ec976dda  guix-build-db895c0e3842/output/x86_64-linux-gnu/SHA256SUMS.part
22dfff0064aadcf516e7d0b46d3203485fbd1d544e512c34fd90c857dc4078f64f  guix-build-db895c0e3842/output/x86_64-linux-gnu/bitcoin-db895c0e3842-x86_64-linux-gnu-debug.tar.gz
234c33c698aca966e106760db4e7a64e585c323c09d548726e4c6d525832d1fbde  guix-build-db895c0e3842/output/x86_64-linux-gnu/bitcoin-db895c0e3842-x86_64-linux-gnu.tar.gz
24064ef4fd2913d9023d24fd63fc55c19eaeb443798376272a863271b1578bd7f2  guix-build-db895c0e3842/output/x86_64-w64-mingw32/SHA256SUMS.part
25504b0aa12c2f0c8dd66e145bd488cf27fd91be0db56e95ac5cb7b279bee3f506  guix-build-db895c0e3842/output/x86_64-w64-mingw32/bitcoin-db895c0e3842-win64-codesigning.tar.gz
267dafb9075618cecacf14c2b8a6c59fa683719c8730d6958d7a659c66ce4fcd96  guix-build-db895c0e3842/output/x86_64-w64-mingw32/bitcoin-db895c0e3842-win64-debug.zip
278d8f88b008b4a51541e9e6f6b7c5fc29035f2b263225ac8d8dfc51a043f861ad  guix-build-db895c0e3842/output/x86_64-w64-mingw32/bitcoin-db895c0e3842-win64-setup-unsigned.exe
28f86084eabf4c2900d7f9732acd6847ac8627ea59e51d82a0c2d6c3a4b2d5c022  guix-build-db895c0e3842/output/x86_64-w64-mingw32/bitcoin-db895c0e3842-win64-unsigned.zip

fs: remove _POSIX_C_SOURCE defining #32460

Code Coverage & Benchmarks

Reviews

fs: remove `_POSIX_C_SOURCE` defining #32460