Skip to content

Unwrap inet_ntop's dst pointer once, for Bionic - #88

Draft
germ-mark wants to merge 1 commit into
apple:mainfrom
germ-network:android-inet-ntop-nullability
Draft

germ-mark wants to merge 1 commit into
apple:mainfrom
germ-network:android-inet-ntop-nullability

Conversation

@germ-mark

@germ-mark germ-mark commented Aug 10, 2026 •

Copy link
Copy Markdown

Motivation

sys_inet_ntop does not compile when building for Android.

Bionic annotates inet_ntop's dst parameter as char * _Nonnull, so Swift imports it
as a non-optional UnsafeMutablePointer<Int8>. Darwin, Glibc and WinSDK leave it
unannotated, so it imports as an optional and passing baseAddress directly type-checks
there.

// arpa/inet.h, Android NDK r27d
const char* _Nullable inet_ntop(int __af, const void* _Nonnull __src, char* _Nonnull __dst, socklen_t __size);

// arpa/inet.h, MacOSX.sdk
const char *inet_ntop(int, const void *, char *_LIBC_COUNT(__size), socklen_t __size);

Building for aarch64-unknown-linux-android28 with the 6.3.3 Android Swift SDK:

Sources/AsyncDNSResolver/c-ares/DNSResolver_c-ares.swift:617:53: error: value of optional type 'UnsafeMutablePointer<Int8>?' must be unwrapped to a value of type 'UnsafeMutablePointer<Int8>'
617 |         if inet_ntop(family, bytes, addressBytesPtr.baseAddress, socklen_t(length)) == nil {
    |                                                     |- error: value of optional type 'UnsafeMutablePointer<Int8>?' must be unwrapped to a value of type 'UnsafeMutablePointer<Int8>'

Modifications

Unwrap addressBytesPtr.baseAddress once with a guard, above the platform branches,
and use the result in both.

This covers the Windows and POSIX calls with one unwrap and replaces the existing
baseAddress! further down, so the function ends up with fewer force-unwraps than
before rather than one more. It also removes the trap that unwrap represented: a
zero-length buffer previously would have crashed there, and now returns nil like any
other failure.

Behaviour is otherwise unchanged — on platforms where dst imports as optional, passing
a non-optional pointer is the same call.

Result

The package builds for Android, and the function no longer force-unwraps at all.

Test Plan

  • swift build and swift test on macOS — clean, 3 tests pass.
  • swift-format lint --strict clean, and swift-format format produces no diff.
  • Cross-compiled for aarch64-unknown-linux-android28 (Swift 6.3.3 host toolchain,
    swift-6.3.3-RELEASE_android.artifactbundle, NDK r27d): fails with the error above
    before this change, builds cleanly after it.

CI has no Android job, so nothing here exercises that path automatically — the change is
inert on every platform CI does cover.

@germ-mark
germ-mark marked this pull request as draft August 10, 2026 22:29
Bionic annotates the dst parameter as `char * _Nonnull`, so it imports as
a non-optional pointer and the call fails to compile in an Android SDK
build. Darwin, Glibc and WinSDK import it as optional.

Hoisting the unwrap above the platform branches fixes both call sites and
removes the existing force-unwrap further down, rather than adding another.
@germ-mark
germ-mark force-pushed the android-inet-ntop-nullability branch from 0791098 to 0c12cea Compare August 10, 2026 22:31
@germ-mark germ-mark changed the title Unwrap inet_ntop's dst pointer for Bionic Unwrap inet_ntop's dst pointer once, for Bionic Aug 10, 2026
@germ-mark

Copy link
Copy Markdown
Author

Apologies, this was prematurely opened against upstream instead of our fork.

But yes, we're trying to compile for Android SDK and ran into this

@glbrntt

glbrntt commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Apologies, this was prematurely opened against upstream instead of our fork.

But yes, we're trying to compile for Android SDK and ran into this

No problem, great to see growing support for Android. Let us know when it's ready for review.

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