Re: Argument type for FS_IOC_GETFLAGS/FS_IOC_SETFLAGS ioctls

[Aurelien Jarno <aurelien@aurel32.net>]

On Wed, Nov 27, 2013 at 08:34:37AM -0500, Theodore Ts'o wrote:
> On Wed, Nov 27, 2013 at 11:03:47AM +0100, Aurelien Jarno wrote:
> > In my case, I am *not* talking about FUSE code, but programs using this
> > ioctl from userland. Changing the kernel FUSE code won't fix the problem
> > I reported.
> 
> I wasn't aware of the fact that there were broken userspace programs
> up until now --- and things have like this for well over a decade,
> with no one complaining until now, which gives you a hint about how
> prevelant this problem actually is.
> 
> > People who do the things correctly lookup the argument type in
> > <linux/fs.h>, they see it's a long and then use a long in their code. And
> > they are right. The bare minimum would be to add a comment close to the
> > definition to explain to use an int and not a long.
> 
> The documentation is specifies the FS_IOC_GETFLAGS and FS_IOC_SETFLAGS
> takes an int * and int, respectively, in the ioctl_list man page.  As

The ioctl_list man page is outdated and only lists the EXT2 equivalent.
Thus people looking for FS_IOC_GETFLAGS or FS_IOC_SETFLAGS might not
find them that easily.

> far as the "self documenting" ioctl numbering is concerned, I've
> always considered it "mostly harmless", but never as authoratative

On the other hand it's the only real documentation available for these
ioctls (see above).

> documentation.  Hence, trying to use words such as "right" or "wrong"
> is not particularly interested as far as I'm concerned.  (I'm reminded
> of the story of the time when Richard Stallman blindly steped into the
> crosswalk, and was almost killed by a car running the red light.
> Someone held him back and said, "didn't you see that car?  He would
> have almost killed you!".  To which Richard said, "but he would have
> been in the wrong.")

I mean people are right to look at the documentation and trust it
instead of trying to guess by looking random pages on the web. The
problem is that the documentation doesn't match the implementation.

> In any case, sure, we can add a documentation to the header file in
> the kernel sources, and the glibc folks will need to be asked to fix
> up /usr/include/linux/fs.h (which is not the same as the
> include/linux/fs.h in the kernel).  But it doesn't change the fact

I have just sent a patch for the kernel sources. Note that <linux/fs.h>
is *not* provided by the glibc, so there is nothing to fix there, but
rather by the kernel as part of the uapi headers.

> that it is bup and libexplain that will need to be changed, regardless
> of whether they were in the "right" or in the "wrong".  If the sense
> of moral superiority makes them more willing to fix their code, fine.

There is no problem to change these as long as it is clear what choice
they should take. And until now the only real way to make the correct
choice was to look at the kernel sources to see how this argument is
read/written.

Regards,
Aurelien

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net