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

[6.0] Android: add better nullability checks for nullability annotations added in NDK 26 #5010

Closed
wants to merge 1 commit into from

Conversation

finagolfin
Copy link
Member

Explanation: This is needed because Bionic recently added a bunch of these annotations.

Scope: Additional nullability checks and force unwraps only

Issue: None

Original PR: This is a cut-down #4850, which will have to be reworked for the swift-foundation merge.

Risk: Low

Testing: I made sure this pull doesn't break anything by testing it with the previous NDK 25c also. I used this patch with others to build the Swift toolchain for my Android CI, finagolfin/swift-android-sdk#122, and the Termux app for Android, which now uses NDK 26b.

Reviewer: @compnerd

@parkera
Copy link
Contributor

parkera commented Jul 17, 2024

We mentioned this in #4850 - but we will be merging that work here, so we'll only need one patch for this in the end.

Comment on lines +582 to +583
if let pwd = getpwuid(s.st_uid), let pwd_name = pwd.pointee.pw_name {
let name = String(cString: pwd_name)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if let pwd = getpwuid(s.st_uid), let pwd_name = pwd.pointee.pw_name {
let name = String(cString: pwd_name)
if let pwd = getpwuid(s.st_uid), let pw_name = pwd.pointee.pw_name {
let name = String(cString: pw_name)

Copy link
Member Author

Choose a reason for hiding this comment

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

Change is fine by me, but as mentioned in #4889, I will get this working with the swift-foundation re-core by next week and re-submit for trunk then.

Copy link
Contributor

Choose a reason for hiding this comment

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

#5011 is the PR to track merging main to release/6.0.

Comment on lines +587 to +588
if let grd = getgrgid(s.st_gid), let grd_name = grd.pointee.gr_name {
let name = String(cString: grd_name)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if let grd = getgrgid(s.st_gid), let grd_name = grd.pointee.gr_name {
let name = String(cString: grd_name)
if let grd = getgrgid(s.st_gid), let gr_name = grd.pointee.gr_name {
let name = String(cString: gr_name)

@compnerd
Copy link
Member

@swift-ci please test

@finagolfin finagolfin marked this pull request as draft July 22, 2024 09:02
@finagolfin
Copy link
Member Author

Some of these are no longer needed after the re-core and other changes, others have been submitted again in #5024 and my pull for that, hyp#1.

@finagolfin finagolfin closed this Aug 11, 2024
@finagolfin finagolfin deleted the release/6.0 branch August 11, 2024 02:51
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.

3 participants