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

Support Device Model #21

Merged
merged 2 commits into from Mar 22, 2024
Merged

Conversation

manucheri
Copy link
Contributor

Add device model information as part of environment info and system properties.

Related to #20

The device model will be reported in the form of "iPhone16,2" (iPhone 15 Pro Max) and similar. This can be mapped to a corresponding model if needed, but that has to be kept up to date when new models are released - so I kept it in this form. That mapping can be done either on the client, or the server, or even the web UI.

Add device model information as part of environment info and system
properties.
var systemInfo = utsname()
uname(&systemInfo)

let identifier = withUnsafePointer(to: &systemInfo.machine) { ptr in
Copy link
Contributor Author

@manucheri manucheri Mar 22, 2024

Choose a reason for hiding this comment

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

We are using pointers here since the API to get the device model is a Unix (C based) API. Would be nice to have a native Swift/Obj-C API for this, but as far as I could find that is not available.

Copy link
Member

Choose a reason for hiding this comment

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

Im ok with that, but is there anyway to safeguard this from an error/exception? I don’t know much about Swift :)

I’d rather the have an empty model than seeing apps crashing because of errors around this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't say I am an expert on the Swift <-> C interop myself.

But basically since we are dealing with C pointers (which are inherently "unsafe") we need to make sure that the memory we are accessing is correct (which it should be due to us simply using the Unix APIs here to read the data), and not messed with, to avoid undefined behavior.

The usage of withUnsafePointer ensures we only read, and will not manipulate, the pointer. What we might get - I guess - if something changes the structure or behavior of utsname (system info) is "unexpected behavior". But since we are not doing that we should be safe.

I'll add a check that uname returned successfully (which if I remember correctly from C is a return value of 0). I also think due to this being a static function called only once when we initialise the library we won't have the issue with something accessing the pointer at the same time.
So I don't really see the need for additional error checking here (famous last words maybe 😅).

@goenning goenning merged commit 9d67448 into aptabase:main Mar 22, 2024
4 checks passed
@goenning
Copy link
Member

Thanks! Let's ship it!

@manucheri manucheri deleted the 20240322-support-devicemodel branch April 15, 2024 07:01
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.

None yet

2 participants