Skip to content

Conversation

@ElectrodeYT
Copy link
Member

os-test shows we don't define these even though we support everything needed for them.

@no92
Copy link
Member

no92 commented Nov 16, 2025

The comment style is not correct. Also, are these macros supposed to be defined to the posix version or just some numeric value?

@ElectrodeYT
Copy link
Member Author

The comment style is not correct. Also, are these macros supposed to be defined to the posix version or just some numeric value?

As far as I can tell, has to be at least some not-zero number, but internet says its commonly defined to the POSIX version,
for comment style, how do you think it could be done while ideally keeping what functions/features are needed for each macro? Might be useful when going through them in the future if we decide to add things such as only enabling the ones where the sysdeps required for each function are present

@no92
Copy link
Member

no92 commented Nov 16, 2025

The comment style should be /* comment */ to work on C89 in pedantic mode.

os-test shows we don't define these even though we support everything needed for them.
@ElectrodeYT ElectrodeYT force-pushed the posix-feature-test-macros branch from 6025571 to 31e117b Compare November 16, 2025 22:49
@ElectrodeYT
Copy link
Member Author

The comment style should be /* comment */ to work on C89 in pedantic mode.

Fixed

Copy link
Member

@no92 no92 left a comment

Choose a reason for hiding this comment

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

From the spec:
_POSIX_MEMLOCK, _POSIX_MEMLOCK_RANGE and _POSIX_SYNCHRONIZED_IO need to be defined to a value greater than zero, and the other should be defined to _POSIX_VERSION of at least POSIX 2024.

LGTM

@no92
Copy link
Member

no92 commented Nov 17, 2025

One line from the spec is a bit annoying:

Runtime checks with fpathconf( ), pathconf( ), or sysconf shall indicate that the option is supported.
This applies to _POSIX_MEMLOCK, _POSIX_MEMLOCK_RANGE and _POSIX_SYNCHRONIZED_IO.

@no92 no92 added this pull request to the merge queue Nov 17, 2025
Merged via the queue into managarm:master with commit 467e5d5 Nov 17, 2025
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants