On Fri, Aug 22, 2025 at 12:54:01PM +0000, Bernd Schubert wrote:
> On 8/22/25 02:33, Joanne Koong wrote:
> > On Wed, Aug 20, 2025 at 6:01 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
> >>
> >> From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >>
> >> fuse.h and fuse_lowlevel.h are public headers, don't expose internal
> >> build system config variables to downstream clients. This can also lead
> >> to function pointer ordering issues if (say) libfuse gets built with
> >> HAVE_STATX but the client program doesn't define a HAVE_STATX.
> >>
> >> Get rid of the conditionals in the public header files to fix this.
> >>
> >> Signed-off-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
> >> ---
> >> include/fuse.h | 2 --
> >> include/fuse_lowlevel.h | 2 --
> >> example/memfs_ll.cc | 2 +-
> >> example/passthrough.c | 2 +-
> >> example/passthrough_fh.c | 2 +-
> >> example/passthrough_ll.c | 2 +-
> >> 6 files changed, 4 insertions(+), 8 deletions(-)
> >>
> >>
> >> diff --git a/include/fuse.h b/include/fuse.h
> >> index 06feacb070fbfb..209102651e9454 100644
> >> --- a/include/fuse.h
> >> +++ b/include/fuse.h
> >> @@ -854,7 +854,6 @@ struct fuse_operations {
> >> */
> >> off_t (*lseek) (const char *, off_t off, int whence, struct fuse_file_info *);
> >>
> >> -#ifdef HAVE_STATX
> >> /**
> >> * Get extended file attributes.
> >> *
> >> @@ -865,7 +864,6 @@ struct fuse_operations {
> >> */
> >> int (*statx)(const char *path, int flags, int mask, struct statx *stxbuf,
> >> struct fuse_file_info *fi);
> >> -#endif
> >> };
> >
> > Are we able to just remove this ifdef? Won't this break compilation on
> > old systems that don't recognize "struct statx"?
>
> Yeah, you had added forward declaration actually. Slipped through in
> my review that we don't need the HAVE_STATX anymore.
>
> We can also extend the patch a bit to remove HAVE_STATX from the public
> config.
> Another alternative for this patch would be to replace HAVE_STATX by
> HAVE_FUSE_STATX.
> The commit message is also not entirely right, as it says
<shrug> libfuse itself doesn't define a struct statx, so what does it
have, aside from the incomplete struct declaration? TBH I wonder what
will happen when struct statx grows, but everybody gets to deal with
that problem because we didn't explicitly encode the size in either the
syscall or the struct definition.
Presumably fuse servers will detect and set their own HAVE_STATX,
and only supply a ->statx function if they HAVE_STATX. They don't have
to know if libfuse itself got built with statx support; if it didn't,
then nothing will ever call ->statx, afaict.
> > to function pointer ordering issues if (say) libfuse gets built with
> > HAVE_STATX but the client program doesn't define a HAVE_STATX.
>
> Actually not, because /usr/include/fuse3/libfuse_config.h defines HAVE_STATX.
> I'm more worried that there might be a conflict of HAVE_STATX from libfuse
> with HAVE_STATX from the application.
<nod>
--D
>
> Thanks,
> Bernd
