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

fuse_session_custom_io: Add a struct size field #930

Closed
wants to merge 1 commit into from

Conversation

bsbernd
Copy link
Collaborator

@bsbernd bsbernd commented Apr 18, 2024

As in commit 73cd124 'struct fuse_custom_io' might get extended and in order to stay ABI compatible libfuse has to zero all extended fields. In order to do so it has to know the size of the struct the application knows about.

As in commit 73cd124 'struct fuse_custom_io' might
get extended and in order to stay ABI compatible libfuse
has to zero all extended fields. In order to do so it has
to know the size of the struct the application knows about.

Signed-off-by: Bernd Schubert <bernd.schubert@fastmail.fm>
Copy link
Collaborator

@ashleypittman ashleypittman left a comment

Choose a reason for hiding this comment

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

This looks good.

As I say it might be possible to support two versions in the same way we do at
https://github.com/libfuse/libfuse/blob/master/include/fuse_lowlevel.h#L1979-L1992 but I'd need to look into that. This solves the problem for master and going forward at least.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Apr 18, 2024

Yeah, we can make it versioned (ABI and API). I cannot work on that right now, probably earliest tomorrow morning. I would also happily let you do the versioning part - if you want :)

@ashleypittman
Copy link
Collaborator

Yeah, we can make it versioned (ABI and API). I cannot work on that right now, probably earliest tomorrow morning. I would also happily let you do the versioning part - if you want :)

sorry I missed this. I'll have a go in the morning.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Apr 22, 2024

Thanks Ashley! My patch to get the libfuse version passed into fuse_session_new is growing and got me busy for spare time over the weekend. Difficulty is the highlevel API. I hope I'm close to get that done, but then didn't have any time for this issue here.

@legezywzh
Copy link
Contributor

This patch looks good to me, thanks for your time. I didn't realize this compatibility issue before, sorry.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Apr 25, 2024

@legezywzh The patch is unfortunately not complete - we need to ensure API and ABI compatibility and that makes it more complex.

@legezywzh
Copy link
Contributor

@legezywzh The patch is unfortunately not complete - we need to ensure API and ABI compatibility and that makes it more complex.

So the correct solution is #935, right? I'll take a look.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Apr 25, 2024

Yeah, but it is not complete yet.

@bsbernd
Copy link
Collaborator Author

bsbernd commented Jun 4, 2024

Already addressed here: #953

@bsbernd bsbernd closed this Jun 4, 2024
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

3 participants