Skip to content

Process: unwrap the posix_spawnattr_t on Android #5179

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

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

compnerd
Copy link
Member

This is required as while posix_spawnattr_init permits a nullable type, posix_spawnattr_setflags properly expects a non-null parameter. Unwrap the newly minted spawnattr or abort if the allocation failed.

This is required as while `posix_spawnattr_init` permits a nullable
type, `posix_spawnattr_setflags` properly expects a non-null parameter.
Unwrap the newly minted spawnattr or abort if the allocation failed.
@compnerd
Copy link
Member Author

CC: @finagolfin

The revert was incorrect, the unwrapping should always be present. However, the position of the unwrapping was incorrect. Even with the older SDK, the unwrapping should occur after the posix_spawnattr_init call. I worked with Google to get the nullability corrected on that call, however, in the mean time, I had overridden the API with APINotes to ensure that we could get the correct code. It seems that the unwrapping was just a bit earlier than it should've been.

This has only been tested with r27c, but I expect it to be backwards compatible with r26b and the Android SDK that we are currently shipping on main.

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

@swift-ci please test Windows platform

Copy link
Member

@finagolfin finagolfin left a comment

Choose a reason for hiding this comment

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

Right, this was always my point, that the guard made no sense right after setting this to nil, whereas this should work.

@compnerd compnerd merged commit 54c4e51 into swiftlang:main Feb 28, 2025
2 checks passed
@compnerd compnerd deleted the spawn branch February 28, 2025 02:51
hjyamauchi added a commit that referenced this pull request Mar 5, 2025
This is required as while `posix_spawnattr_init` permits a nullable
type, `posix_spawnattr_setflags` properly expects a non-null parameter.
Unwrap the newly minted spawnattr or abort if the allocation failed.

Cherry pick commit #5179
finagolfin pushed a commit that referenced this pull request Mar 6, 2025
This is required as while `posix_spawnattr_init` permits a nullable
type, `posix_spawnattr_setflags` properly expects a non-null parameter.
Unwrap the newly minted spawnattr or abort if the allocation failed.

Cherry pick commit #5179
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