On Tue, Feb 18, 2020 at 04:58:47PM -0800, Darrick J. Wong wrote:
> On Tue, Feb 18, 2020 at 04:39:24PM +0100, Christoph Hellwig wrote:
> > On Tue, Feb 18, 2020 at 10:53:16AM +1100, Dave Chinner wrote:
> > > > diff --git a/fs/xfs/libxfs/xfs_fs.h b/fs/xfs/libxfs/xfs_fs.h
> > > > index ae77bcd8c05b..21920f613d42 100644
> > > > --- a/fs/xfs/libxfs/xfs_fs.h
> > > > +++ b/fs/xfs/libxfs/xfs_fs.h
> > > > @@ -597,7 +597,7 @@ typedef struct xfs_fsop_attrlist_handlereq {
> > > > struct xfs_attrlist_cursor pos; /* opaque cookie, list offset */
> > > > __u32 flags; /* which namespace to use */
> > > > __u32 buflen; /* length of buffer supplied */
> > > > - void __user *buffer; /* returned names */
> > > > + struct xfs_attrlist __user *buffer;/* returned names */
> > > > } xfs_fsop_attrlist_handlereq_t;
> > >
> > > This changes the userspace API, right? So, in theory, it could break
> > > compilation of userspace applications that treat it as an attrlist_t
> > > and don't specifically cast the assignment because it's currently
> > > a void pointer?
> >
> > IFF userspace was using this header it would change the API. But
> > userspace uses the libattr definition exclusively.
>
> Assuming most userspace will use libhandle (and not call the ioctl
> directly) then this "shouldn't" be a problem because libhandle treats
> the attrlist buffer as a void pointer.
There's a lot of "if's" there. :/
All I'm asking for is that the changes are documented as known and
intentional so that we don't end up a couple of years down the track
wondering WTF we were thinking when we made this change...
> (I dunno, how difficult /is/ it to say "program to the library, not the
> kernel ABI" here?)
The xfsctl(3) man page already says this:
XFS_IOC_PATH_TO_HANDLE
XFS_IOC_PATH_TO_FSHANDLE
XFS_IOC_FD_TO_HANDLE
XFS_IOC_OPEN_BY_HANDLE
XFS_IOC_READLINK_BY_HANDLE
XFS_IOC_ATTR_LIST_BY_HANDLE
XFS_IOC_ATTR_MULTI_BY_HANDLE
XFS_IOC_FSSETDM_BY_HANDLE
These are all interfaces that are used to implement
various libhandle functions (see open_by_handle(3)).
They are all subject to change and should not be
called directly by applications.
and the open_by_handle(3) man page says "use libhandle to access
these functions".
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
