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

Adds bindins to the "iio_device_get_label()". #25

Closed

Conversation

AntoineZenKraken
Copy link

This should solve #24.

@fpagliughi
Copy link
Owner

Hey, thanks for the PR. I'm going to go through the API and look for anything else missing to get into a v0.6 release in a few weeks. Let me know if you notice anything else that should be added.

@fpagliughi fpagliughi added this to the v0.6 milestone Feb 3, 2023
@fpagliughi
Copy link
Owner

Oh, actually, you got rid of the 32-bit bindings? Did you test this on a 32-bit system, like an ARMv7 to make sure you didn't break those builds?

@fpagliughi
Copy link
Owner

I'm going to close this due to the lack of separate bindings for 64-bit and 32-bit systems. IIRC, these were needed to deal with the different pointer sizes in the C structs. If they're the wrong size, the bindings are mis-aligned, and you get a segfault.

I leap-frog'ed the version support to libiio v0.24, but made it conditional based on this or the last one (v0.21). If you needed v0.23 for a specific reason, it would be trivial to add.

And I started to add support for the new functions in the latest version, making them dependent on the selected bindings, like:

    /// Gets the label of the device, if any.
    #[cfg(feature = "libiio_v0_24")]
    pub fn label(&self) -> Option<String> {
        let pstr = unsafe { ffi::iio_device_get_label(self.dev) };
        cstring_opt(pstr)
    }

@AntoineZenKraken
Copy link
Author

Oh, actually, you got rid of the 32-bit bindings? Did you test this on a 32-bit system, like an ARMv7 to make sure you didn't break those builds?

No, I have no mean of testing this. I was wondering why there was two bindings. Because on my host, when I generate the 32bits and 64 bits bindings, they are exactly the same. I trough there was an issue with an earlier version of cargo-bindgen but not sure.

@AntoineZenKraken
Copy link
Author

I'm going to close this due to the lack of separate bindings for 64-bit and 32-bit systems. IIRC, these were needed to deal with the different pointer sizes in the C structs. If they're the wrong size, the bindings are mis-aligned, and you get a segfault.

I leap-frog'ed the version support to libiio v0.24, but made it conditional based on this or the last one (v0.21). If you needed v0.23 for a specific reason, it would be trivial to add.

And I started to add support for the new functions in the latest version, making them dependent on the selected bindings, like:

    /// Gets the label of the device, if any.
    #[cfg(feature = "libiio_v0_24")]
    pub fn label(&self) -> Option<String> {
        let pstr = unsafe { ffi::iio_device_get_label(self.dev) };
        cstring_opt(pstr)
    }

I'm perfeclty fine with that! Thanks for your support fpagliughi!

@fpagliughi
Copy link
Owner

Yeah, the problem with the bindings is mostly when you have them in C structs going across the boundary, like:

struct {
  void* some_ptr;
  uint16_t some_small_int;
  ...
};

WIth 64-bit bindings, some_small_int will be at offset 8 in the struct. With 32-bit bindings, it'll be at offset 4. So if you use 64-bit bindings on, say, a 32-bit ARMv7, you'll crash with a segfault.

Honestly, I can't remember if there's anything in the libiio API like this, but I've been burned on a number of other wrapper-type Rust projects, so I figured I'd play it safe here as well.

But in order to generate the 32=bit bindings, you have to do it on a 32-bit host... or do it in a cross-compiled environment. I just do it natively on an old Nvidia TK1 board.

@AntoineZenKraken
Copy link
Author

Ho, I trough that the #[repr(C)] was handling alignment issue at compile time on the Rust side... But I have no experience yet on Rust to C binding, so I trust you. I close this PR now.

@fpagliughi
Copy link
Owner

It does... but the pointers are simply different sizes on different platforms!
On a 32-bit system, a pointer is 4-bytes, aligned to a 4-byte boundary.
On a 64-bit system, a pointer is 8-bytes, aligned to an 8-byte boundary.

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