Make the changes to custom_io backwards compabible. #935
Conversation
ashleypittman
force-pushed
the
custom-io-ep
branch
from
4162f51
to
4a538c1
Compare
| #else | ||
| int fuse_session_custom_io_317(struct fuse_session *se, | ||
| const struct fuse_custom_io *io, size_t io_size, int fd); | ||
| #define fuse_session_custom_io(_SE, _IO, _FD) fuse_session_custom_io_317(_SE, _IO, (sizeof(_IO)), _FD) |
There was a problem hiding this comment.
This makes it API compatible, but redirects with macro to the fixed function name _317. To make it ABI compatible with versioned symbols, like this
int fuse_session_loop_mt_312(struct fuse_session *se, struct fuse_loop_config *config);
FUSE_SYMVER("fuse_session_loop_mt_312", "fuse_session_loop_mt@@FUSE_3.12")
int fuse_session_loop_mt_312(struct fuse_session *se, struct fuse_loop_config *config)
There was a problem hiding this comment.
I think this is probably correct as-is but I agree this should use versioned symbols, I'm still trying to get my head around them completely.
There was a problem hiding this comment.
Are you sure? The FUSE_SYMVER macro is not available to the public headers.
There was a problem hiding this comment.
This is a libfuse internal file, so it doesn't need to be in public headers. The public header basically has to define the API for
#if LIBFUSE_BUILT_WITH_VERSIONED_SYMBOLS
#if FUSE_USE_VERSION < FUSE_MAKE_VERSION(3,17)
int fuse_session_custom_io(struct fuse_session *se, const struct fuse_custom_io *io,
int fd);
#else
int fuse_session_custom_io(struct fuse_session *se, const struct fuse_custom_io *io,
size_t fuse_custom_io_sz, int fd);
#endif
#else
/* macro solution as you already did */
#endif
There was a problem hiding this comment.
This is the public header.
There was a problem hiding this comment.
Ah now I see what you mean, FUSE_SYMVER() needs to be done inside of the c file, as with the other compat symbols.
| @@ -3116,9 +3116,16 @@ struct fuse_session *fuse_session_new(struct fuse_args *args, | |||
| return NULL; | |||
| } | |||
|
|
|||
| int fuse_session_custom_io(struct fuse_session *se, const struct fuse_custom_io *io, | |||
| int fd) | |||
| int fuse_session_custom_io_317(struct fuse_session *se, const struct fuse_custom_io *io, | |||
There was a problem hiding this comment.
FUSE_SYMVER should appear here.
| return 0; | ||
| } | ||
|
|
||
| #undef fuse_session_custom_io | ||
|
|
||
| int fuse_session_custom_io(struct fuse_session *se, |
There was a problem hiding this comment.
Just for your information - I'm not sure if needed here, but I had to fuse_parse_cmdline() to compat.c (for libc without versioned symbols) and the ABI symbol in the corresponding c file.
There was a problem hiding this comment.
Just for your information - I'm not sure if needed here, but I had to fuse_parse_cmdline() to compat.c (for libc without versioned symbols) and the ABI symbol in the corresponding c file.
I wonder why fuse_session_loop_mt doesn't need same actions in lib/compat.c.
There was a problem hiding this comment.
See fuse_lowlevel.h
#if (defined(LIBFUSE_BUILT_WITH_VERSIONED_SYMBOLS))
int fuse_parse_cmdline(struct fuse_args *args,
struct fuse_cmdline_opts *opts);
#else
#if FUSE_USE_VERSION < FUSE_MAKE_VERSION(3, 12)
int fuse_parse_cmdline_30(struct fuse_args *args,
struct fuse_cmdline_opts *opts);
#define fuse_parse_cmdline(args, opts) fuse_parse_cmdline_30(args, opts)
#else
int fuse_parse_cmdline_312(struct fuse_args *args,
struct fuse_cmdline_opts *opts);
#define fuse_parse_cmdline(args, opts) fuse_parse_cmdline_312(args, opts)
#endif
#endif
So for LIBFUSE_BUILT_WITH_VERSIONED_SYMBOLS no issue and compat.c not needed. But for the else branch we need to redirect fuse_parse_cmdline() for API to fuse_parse_cmdline_30 and fuse_parse_cmdline_312. For ABI we still need to provide a fuse_parse_cmdline symbol - if defined in the same header this clashes with the macro. I probably could have left fuse_parse_cmdline() inside of fuse_lowlevel.c, but I think it is cleaner to have it in compat.c
Pass in a size field to session_custom_io so that new entries can be added without breaking the ABI. Make the function prototype backwards compatible. Signed-off-by: Ashley Pittman <ashley.m.pittman@intel.com>
ashleypittman
force-pushed
the
custom-io-ep
branch
from
4a538c1
to
72c5cba
Compare
|
@ashleypittman hi, would you like to push a new one according to bsbernd's comments, seems that the only not resolved issue is whether to do same actions as fuse_parse_cmdline in lib/compat.c |
|
@bsbernd @ashleypittman if you don't have free time, do you agree that I take over this pr, do some rebase and rework according to current patch, your previous discussions and libfuse code base? |
|
@legezywzh I would have time for it earliest next week. You might want to look at my recent commit (58f85bf), which also adds new compat symbols. And sure, the earlier we get this compat issue fixed the better - if you have time, you are very welcome to take it over. |
No issue from me. Other than the fact this PR needs merging with master I believe it's right from a C perspective but as per the comments needs extra handling for versioned symbols. This isn't something I've needed to touch before so I need to come up to speed on this which is taking me a while to be able to schedule some time for. If you want to take over this then I have no issue with it. |
|
@bsbernd @ashleypittman since this compatibility issue is introduced by my patch :) I'll prepare a patch this week, thnaks. |
|
Already addressed here: #953 |
Pass in a size field to session_custom_io so that new entries
can be added without breaking the ABI.
Make the function prototype backwards compatible.
Signed-off-by: Ashley Pittman ashley.m.pittman@intel.com