v4l2 core: return -ENOIOCTLCMD if an ioctl doesn't exist

Currently, -EINVAL is used to return either when an IOCTL is not implemented, or if the ioctl was not implemented.

Quoted 2 lines · Mauro Carvalho Chehab

Currently, -EINVAL is used to return either when an IOCTL is not implemented, or if the ioctl was not implemented.

Hi Mauro,

Thanks for the patch.

The V4L2 core probably should return -ENOIOCTLCMD when an IOCTL isn't implemented, but as long as vfs_ioctl() would stay as it is, the user space would still get -EINVAL. Or is vfs_ioctl() about to change?

fs/ioctl.c:
----8<-----------
static long vfs_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)

{
        int error = -ENOTTY;

        if (!filp->f_op || !filp->f_op->unlocked_ioctl)
                goto out;

        error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
        if (error == -ENOIOCTLCMD)
                error = -EINVAL;
 out:
        return error;
}

----8<-----------

Quoted 12 lines

The V4L2 core probably should return -ENOIOCTLCMD when an IOCTL isn't implemented, but as long as vfs_ioctl() would stay as it is, the user space would still get -EINVAL. Or is vfs_ioctl() about to change?

fs/ioctl.c:
----8<-----------
static long vfs_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)

{
        int error = -ENOTTY;

        if (!filp->f_op || !filp->f_op->unlocked_ioctl)
                goto out;

Quoted 7 lines

        error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
        if (error == -ENOIOCTLCMD)
                error = -EINVAL;
 out:
        return error;
}

----8<-----------

Good catch!

At the recent git history, the return for -ENOIOCTLCMD were modified by this changeset:

commit b19dd42faf413b4705d4adb38521e82d73fa4249

Author: Arnd Bergmann <[email protected]>
Date:   Sun Jul 4 00:15:10 2010 +0200

bkl: Remove locked .ioctl file operation
...

(quoted diff) +1 -11

@@ -39,21 +38,12 @@ static long vfs_ioctl(struct file *filp, unsigned int cmd,
 {
        int error = -ENOTTY;
 
-   if (!filp->f_op)
+ if (!filp->f_op || !filp->f_op->unlocked_ioctl)
                goto out;
 
-   if (filp->f_op->unlocked_ioctl) {
-           error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
-           if (error == -ENOIOCTLCMD)
-                   error = -EINVAL;
-           goto out;
-   } else if (filp->f_op->ioctl) {
-           lock_kernel();
-           error = filp->f_op->ioctl(filp->f_path.dentry->d_inode,
-                                     filp, cmd, arg);
-           unlock_kernel();
...

Before Arnd's patch, locked ioctl's were returning -ENOIOCTLCMD, and unlocked ones were returning -EINVAL. Now, the return of -ENOIOCTLCMD doesn't go to userspace anymore. IMO, that's wrong and can cause regressions, as some subsystems like DVB were returning -ENOIOCTLCMD to userspace.

The right fix would be to remove this from fs:

fs/ioctl.c +0 -2

index 1d9b9fc..802fbbd 100644
@@ -41,8 +41,6 @@ static long vfs_ioctl(struct file *filp, unsigned int cmd,
 		goto out;
 
 	error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
-	if (error == -ENOIOCTLCMD)
-		error = -EINVAL;
  out:
 	return error;
 }

However, the replacement from -EINVAL to -ENOIOCTLCMD is there since 2.6.12 for unlocked_ioctl:

$ git blame b19dd42f^1 fs/ioctl.c 
...
^1da177e (Linus Torvalds    2005-04-16 15:20:36 -0700  46)              error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
^1da177e (Linus Torvalds    2005-04-16 15:20:36 -0700  47)              if (error == -ENOIOCTLCMD)
^1da177e (Linus Torvalds    2005-04-16 15:20:36 -0700  48)                      error = -EINVAL;

Linus,

what would be the expected behaviour?

Quoted 13 lines

Quoted 13 lines

The V4L2 core probably should return -ENOIOCTLCMD when an IOCTL isn't implemented, but as long as vfs_ioctl() would stay as it is, the user space would still get -EINVAL. Or is vfs_ioctl() about to change?

fs/ioctl.c:
----8<-----------
static long vfs_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)

{
        int error = -ENOTTY;

        if (!filp->f_op || !filp->f_op->unlocked_ioctl)
                goto out;

Quoted 7 lines

Quoted 7 lines

        error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
        if (error == -ENOIOCTLCMD)
                error = -EINVAL;
 out:
        return error;
}

----8<-----------

One of the differences between the old ->ioctl() and the ->unlocked_ioctl() function is that unlocked_ioctl could point to the same function as ->compat_ioctl(), so we have to catch functions returning -ENOIOCTLCMD.

Quoted 31 lines

Good catch!

At the recent git history, the return for -ENOIOCTLCMD were modified by this changeset:

commit b19dd42faf413b4705d4adb38521e82d73fa4249

Author: Arnd Bergmann <[email protected]>
Date:   Sun Jul 4 00:15:10 2010 +0200

bkl: Remove locked .ioctl file operation
...

(quoted diff) +1 -11

@@ -39,21 +38,12 @@ static long vfs_ioctl(struct file *filp, unsigned int cmd,
 {
        int error = -ENOTTY;
 
-   if (!filp->f_op)
+ if (!filp->f_op || !filp->f_op->unlocked_ioctl)
                goto out;
 
-   if (filp->f_op->unlocked_ioctl) {
-           error = filp->f_op->unlocked_ioctl(filp, cmd, arg);
-           if (error == -ENOIOCTLCMD)
-                   error = -EINVAL;
-           goto out;
-   } else if (filp->f_op->ioctl) {
-           lock_kernel();
-           error = filp->f_op->ioctl(filp->f_path.dentry->d_inode,
-                                     filp, cmd, arg);
-           unlock_kernel();
...

Quoted 5 lines

Before Arnd's patch, locked ioctl's were returning -ENOIOCTLCMD, and unlocked ones were returning -EINVAL. Now, the return of -ENOIOCTLCMD doesn't go to userspace anymore. IMO, that's wrong and can cause regressions, as some subsystems like DVB were returning -ENOIOCTLCMD to userspace.

ENOIOCTLCMD should never be returned to user space, see the comment in include/linux/errno.h:

/*
* These should never be seen by user programs. To return one of ERESTART*
* codes, signal_pending() MUST be set. Note that ptrace can observe these
* at syscall exit tracing, but they will never be left for the debugged user
* process to see.
*/

There was a lot of debate whether undefined ioctls on non-ttys should return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY to -EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(), which turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you observe where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Quoted 3 lines

Linus,

what would be the expected behaviour?

Note that 1da177e is the initial commit to git, Linus did not write that code, although he might have an opinion.

Quoted 8 lines

There was a lot of debate whether undefined ioctls on non-ttys should return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY to -EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(), which turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you observe where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Well, currently, it is returning -EINVAL maybe due to the mass-conversions you've mentioned.

The point is that -EINVAL has too many meanings at V4L. It currently can be either that an ioctl is not supported, or that one of the parameters had an invalid parameter. If the userspace can't distinguish between an unimplemented ioctl and an invalid parameter, it can't decide if it needs to fall back to some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not implemented.

Quoted 9 lines

Quoted 8 lines

There was a lot of debate whether undefined ioctls on non-ttys should return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY to -EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(), which turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you observe where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Quoted 2 lines

Well, currently, it is returning -EINVAL maybe due to the mass-conversions you've mentioned.

I mean what do you return *to* vfs_ioctl from v4l? The conversions must have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD into EINVAL, so changing the vfs functions would not have any effect.

Quoted 7 lines

The point is that -EINVAL has too many meanings at V4L. It currently can be either that an ioctl is not supported, or that one of the parameters had an invalid parameter. If the userspace can't distinguish between an unimplemented ioctl and an invalid parameter, it can't decide if it needs to fall back to some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not implemented.

That is what a lot of subsystems do these days. But wouldn't that change your ABI?

Quoted 6 lines

I mean what do you return to vfs_ioctl from v4l? The conversions must have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD into EINVAL, so changing the vfs functions would not have any effect.

Yes. This discussion was originated by a RFC patch proposing to change video_ioctl2 to return -ENOIOCTLCMD instead of -EINVAL.

Em 26-06-2011 15:20, Arnd Bergmann escreveu:

Quoted 10 lines · On Sunday 26 June 2011 19:30:46 Mauro Carvalho Chehab wrote:

Quoted 9 lines

Quoted 8 lines

There was a lot of debate whether undefined ioctls on non-ttys should return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY to -EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(), which turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you observe where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Quoted 8 lines

Quoted 2 lines

Well, currently, it is returning -EINVAL maybe due to the mass-conversions you've mentioned.

I mean what do you return *to* vfs_ioctl from v4l? The conversions must have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD into EINVAL, so changing the vfs functions would not have any effect.

Yes. This discussion was originated by a RFC patch proposing to change video_ioctl2 to return -ENOIOCTLCMD instead of -EINVAL.

Quoted 8 lines

Quoted 7 lines

The point is that -EINVAL has too many meanings at V4L. It currently can be either that an ioctl is not supported, or that one of the parameters had an invalid parameter. If the userspace can't distinguish between an unimplemented ioctl and an invalid parameter, it can't decide if it needs to fall back to some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not implemented.

Quoted 2 lines

That is what a lot of subsystems do these days. But wouldn't that change your ABI?

Yes. The patch in question is also changing the DocBook spec for the ABI. We'll likely need to drop some notes about that at the features-to-be-removed.txt.

I don't think that applications are relying at -EINVAL in order to detect if an ioctl is not supported, but before merging such patch, we need to double-check.

Quoted 31 lines · On Sunday, June 26, 2011 20:51:37 Mauro Carvalho Chehab wrote:

Em 26-06-2011 15:20, Arnd Bergmann escreveu:

Quoted 10 lines · On Sunday 26 June 2011 19:30:46 Mauro Carvalho Chehab wrote:

Quoted 9 lines

Quoted 8 lines

There was a lot of debate whether undefined ioctls on non-ttys should return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY to -EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(), which turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you observe where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Quoted 8 lines

Quoted 2 lines

Well, currently, it is returning -EINVAL maybe due to the mass-conversions you've mentioned.

I mean what do you return *to* vfs_ioctl from v4l? The conversions must have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD into EINVAL, so changing the vfs functions would not have any effect.

Yes. This discussion was originated by a RFC patch proposing to change video_ioctl2 to return -ENOIOCTLCMD instead of -EINVAL.

Quoted 8 lines

Quoted 7 lines

The point is that -EINVAL has too many meanings at V4L. It currently can be either that an ioctl is not supported, or that one of the parameters had an invalid parameter. If the userspace can't distinguish between an unimplemented ioctl and an invalid parameter, it can't decide if it needs to fall back to some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not implemented.

Quoted 8 lines

Quoted 2 lines

That is what a lot of subsystems do these days. But wouldn't that change your ABI?

Yes. The patch in question is also changing the DocBook spec for the ABI. We'll likely need to drop some notes about that at the features-to-be-removed.txt.

I don't think that applications are relying at -EINVAL in order to detect if an ioctl is not supported, but before merging such patch, we need to double-check.

I really don't think we can change this behavior. It's been part of the spec since forever and it is not just open source apps that can rely on this, but also closed source. Making an ABI change like this can really mess up applications.

We should instead review the spec and ensure that applications can discover what is and what isn't supported through e.g. capabilities.

Hi Hans,

Quoted 65 lines · Hans Verkuil

Quoted 3 lines · On Sunday, June 26, 2011 20:51:37 Mauro Carvalho Chehab wrote:

Em 26-06-2011 15:20, Arnd Bergmann escreveu:

Quoted 2 lines · On Sunday 26 June 2011 19:30:46 Mauro Carvalho Chehab wrote:

Quoted 1 line

Quoted 1 line

There was a lot of debate whether undefined ioctls on non-ttys

should

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY

to

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

-EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(),

which

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you

observe

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Well, currently, it is returning -EINVAL maybe due to the

mass-conversions

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

you've mentioned.

I mean what do you return *to* vfs_ioctl from v4l? The conversions

must

Quoted 3 lines

Quoted 3 lines

have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD

into

Quoted 3 lines

Quoted 1 line

EINVAL, so changing the vfs functions would not have any effect.

Yes. This discussion was originated by a RFC patch proposing to

change

Quoted 3 lines

video_ioctl2 to return -ENOIOCTLCMD instead of -EINVAL.

Quoted 1 line

Quoted 1 line

The point is that -EINVAL has too many meanings at V4L. It

currently can be

Quoted 1 line

Quoted 1 line

Quoted 1 line

either that an ioctl is not supported, or that one of the

parameters had

Quoted 1 line

Quoted 1 line

Quoted 1 line

an invalid parameter. If the userspace can't distinguish between an

unimplemented

Quoted 1 line

Quoted 1 line

Quoted 1 line

ioctl and an invalid parameter, it can't decide if it needs to fall

back to

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not

implemented.

Quoted 2 lines

Quoted 2 lines

That is what a lot of subsystems do these days. But wouldn't that

change

Quoted 3 lines

Quoted 1 line

your ABI?

Yes. The patch in question is also changing the DocBook spec for the

ABI. We'll

Quoted 1 line

likely need to drop some notes about that at the

features-to-be-removed.txt.

Quoted 2 lines

I don't think that applications are relying at -EINVAL in order to

detect if

Quoted 1 line

an ioctl is not supported, but before merging such patch, we need to

double-check.

I really don't think we can change this behavior. It's been part of the spec since
forever and it is not just open source apps that can rely on this, but also closed
source. Making an ABI change like this can really mess up applications.

Quoted 3 lines

We should instead review the spec and ensure that applications can discover what
is and what isn't supported through e.g. capabilities.

As far as I understand, V4L2 wouldn't be the only kernel API to use ENOTTY to tell that an ioctl doesn't exist; there are others. And many switched from EINVAL they used in the past. From that point it would be good to do it
on V4L2 as well. Although I have to reckon that the V4L2 API does serve use
cases of quite different natures than these --- I can't think of an equivalent e.g. to that astronomy application using V4L1 in the scope of these:

Examples:
- Networking
- KVM
- SCSI/libata-scsi

Currently EINVAL is used to signal from a phletora of conditions in V4L2, usually bad, in a way or another, parameters to an ioctl. The more low level
APIs we add (for cameras, for example), the less guessing of parameters can
be done in general. I think it would be important to distinguish the two cases and we don't have enumeration capability (do we?) to tell which IOCTLs
the application should be expect to be able to use.

Interestingly enough, V4L2 core (v4l2_ioctl() in v4l2-dev.c) does return ENOTTY *right now* when the IOCTL handler is not defined. Have we heard about this up to now? :-)

As you mention, switching to ENOTTY in general would change the ABI which would potentially break applications. Can this be handled in a way or another? My understanding is that not many applications would rely on EINVAL
telling an IOCTL isn't implemented. GStreamer v4l2src might be one in its attempt to figure out what kind of image sizes the device supports. Fixing this would be a very small change.

In short, I think it would be beneficial to switch to ENOTTY in the long run even if it causes some momentary pain.

Quoted 87 lines

Hi Hans,

Quoted 65 lines · Hans Verkuil

Quoted 3 lines · On Sunday, June 26, 2011 20:51:37 Mauro Carvalho Chehab wrote:

Em 26-06-2011 15:20, Arnd Bergmann escreveu:

Quoted 2 lines · On Sunday 26 June 2011 19:30:46 Mauro Carvalho Chehab wrote:

Quoted 1 line

Quoted 1 line

There was a lot of debate whether undefined ioctls on non-ttys

should

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY

to

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

-EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(),

which

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you

observe

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Well, currently, it is returning -EINVAL maybe due to the

mass-conversions

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

you've mentioned.

I mean what do you return *to* vfs_ioctl from v4l? The conversions

must

Quoted 3 lines

Quoted 3 lines

have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD

into

Quoted 3 lines

Quoted 1 line

EINVAL, so changing the vfs functions would not have any effect.

Yes. This discussion was originated by a RFC patch proposing to

change

Quoted 3 lines

video_ioctl2 to return -ENOIOCTLCMD instead of -EINVAL.

Quoted 1 line

Quoted 1 line

The point is that -EINVAL has too many meanings at V4L. It

currently can be

Quoted 1 line

Quoted 1 line

Quoted 1 line

either that an ioctl is not supported, or that one of the

parameters had

Quoted 1 line

Quoted 1 line

Quoted 1 line

an invalid parameter. If the userspace can't distinguish between an

unimplemented

Quoted 1 line

Quoted 1 line

Quoted 1 line

ioctl and an invalid parameter, it can't decide if it needs to fall

back to

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not

implemented.

Quoted 2 lines

Quoted 2 lines

That is what a lot of subsystems do these days. But wouldn't that

change

Quoted 3 lines

Quoted 1 line

your ABI?

Yes. The patch in question is also changing the DocBook spec for the

ABI. We'll

Quoted 1 line

likely need to drop some notes about that at the

features-to-be-removed.txt.

Quoted 2 lines

I don't think that applications are relying at -EINVAL in order to

detect if

Quoted 1 line

an ioctl is not supported, but before merging such patch, we need to

double-check.

I really don't think we can change this behavior. It's been part of the spec since
forever and it is not just open source apps that can rely on this, but also closed
source. Making an ABI change like this can really mess up applications.

Quoted 3 lines

We should instead review the spec and ensure that applications can discover what
is and what isn't supported through e.g. capabilities.

As far as I understand, V4L2 wouldn't be the only kernel API to use ENOTTY to tell that an ioctl doesn't exist; there are others. And many switched from EINVAL they used in the past. From that point it would be good to do it
on V4L2 as well. Although I have to reckon that the V4L2 API does serve use
cases of quite different natures than these --- I can't think of an equivalent e.g. to that astronomy application using V4L1 in the scope of these:

Examples:
- Networking
- KVM
- SCSI/libata-scsi

Quoted 9 lines

Currently EINVAL is used to signal from a phletora of conditions in V4L2, usually bad, in a way or another, parameters to an ioctl. The more low level
APIs we add (for cameras, for example), the less guessing of parameters can
be done in general. I think it would be important to distinguish the two cases and we don't have enumeration capability (do we?) to tell which IOCTLs
the application should be expect to be able to use.

While we don't have an enum capability, in many cases you can deduce whether a particular ioctl should be supported or not. Usually based on capabilities, sometimes because certain ioctls allow 'NOP' operations that allow you to test for their presence.

Of course, drivers are not always consistent here, but that's a separate problem.

Quoted 3 lines

Interestingly enough, V4L2 core (v4l2_ioctl() in v4l2-dev.c) does return ENOTTY *right now* when the IOCTL handler is not defined. Have we heard about this up to now? :-)

No, but that's because all drivers have an ioctl handler :-) So you never see ENOTTY.

Quoted 9 lines

As you mention, switching to ENOTTY in general would change the ABI which would potentially break applications. Can this be handled in a way or another? My understanding is that not many applications would rely on EINVAL
telling an IOCTL isn't implemented. GStreamer v4l2src might be one in its attempt to figure out what kind of image sizes the device supports. Fixing this would be a very small change.

Quoted 2 lines

In short, I think it would be beneficial to switch to ENOTTY in the long run even if it causes some momentary pain.

I would like that as well, but the V4L2 Specification explicitly mentions EINVAL as the error code if an ioctl is not supported. It has done so since it was created. You cannot just change that. And closed source programs may very well rely on this.

I don't think changing such an important return value is acceptable.

A better approach would be to allow applications to deduce whether ioctls are (should be) present or not based on capabilities, etc. And document that in the spec and ensure that drivers do this right.

The v4l2-compliance tool is already checking that where possible.

Regards,

Hans

Em 27-06-2011 09:17, Hans Verkuil escreveu:

Quoted 102 lines

Quoted 87 lines

Hi Hans,

Quoted 65 lines · Hans Verkuil

Quoted 3 lines · On Sunday, June 26, 2011 20:51:37 Mauro Carvalho Chehab wrote:

Em 26-06-2011 15:20, Arnd Bergmann escreveu:

Quoted 2 lines · On Sunday 26 June 2011 19:30:46 Mauro Carvalho Chehab wrote:

Quoted 1 line

Quoted 1 line

There was a lot of debate whether undefined ioctls on non-ttys

should

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

return -EINVAL or -ENOTTY, including mass-conversions from -ENOTTY

to

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

-EINVAL at some point in the pre-git era, IIRC.

Inside of v4l2, I believe this is handled by video_usercopy(),

which

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

turns the driver's -ENOIOCTLCMD into -ENOTTY. What cases do you

observe

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

Quoted 2 lines

where this is not done correctly and we do return ENOIOCTLCMD to vfs_ioctl?

Well, currently, it is returning -EINVAL maybe due to the

mass-conversions

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

you've mentioned.

I mean what do you return *to* vfs_ioctl from v4l? The conversions

must

Quoted 3 lines

Quoted 3 lines

have been long before we introduced compat_ioctl and ENOIOCTLCMD.

As far as I can tell, video_ioctl2 has always converted ENOIOCTLCMD

into

Quoted 3 lines

Quoted 1 line

EINVAL, so changing the vfs functions would not have any effect.

Yes. This discussion was originated by a RFC patch proposing to

change

Quoted 3 lines

video_ioctl2 to return -ENOIOCTLCMD instead of -EINVAL.

Quoted 1 line

Quoted 1 line

The point is that -EINVAL has too many meanings at V4L. It

currently can be

Quoted 1 line

Quoted 1 line

Quoted 1 line

either that an ioctl is not supported, or that one of the

parameters had

Quoted 1 line

Quoted 1 line

Quoted 1 line

an invalid parameter. If the userspace can't distinguish between an

unimplemented

Quoted 1 line

Quoted 1 line

Quoted 1 line

ioctl and an invalid parameter, it can't decide if it needs to fall

back to

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

some different methods of handling a V4L device.

Maybe the answer would be to return -ENOTTY when an ioctl is not

implemented.

Quoted 2 lines

Quoted 2 lines

That is what a lot of subsystems do these days. But wouldn't that

change

Quoted 3 lines

Quoted 1 line

your ABI?

Yes. The patch in question is also changing the DocBook spec for the

ABI. We'll

Quoted 1 line

likely need to drop some notes about that at the

features-to-be-removed.txt.

Quoted 2 lines

I don't think that applications are relying at -EINVAL in order to

detect if

Quoted 1 line

an ioctl is not supported, but before merging such patch, we need to

double-check.

I really don't think we can change this behavior. It's been part of the spec since
forever and it is not just open source apps that can rely on this, but also closed
source. Making an ABI change like this can really mess up applications.

Quoted 3 lines

We should instead review the spec and ensure that applications can discover what
is and what isn't supported through e.g. capabilities.

As far as I understand, V4L2 wouldn't be the only kernel API to use ENOTTY to tell that an ioctl doesn't exist; there are others. And many switched from EINVAL they used in the past. From that point it would be good to do it
on V4L2 as well. Although I have to reckon that the V4L2 API does serve use
cases of quite different natures than these --- I can't think of an equivalent e.g. to that astronomy application using V4L1 in the scope of these:

Examples:
- Networking
- KVM
- SCSI/libata-scsi

Quoted 9 lines

Currently EINVAL is used to signal from a phletora of conditions in V4L2, usually bad, in a way or another, parameters to an ioctl. The more low level
APIs we add (for cameras, for example), the less guessing of parameters can
be done in general. I think it would be important to distinguish the two cases and we don't have enumeration capability (do we?) to tell which IOCTLs
the application should be expect to be able to use.

While we don't have an enum capability, in many cases you can deduce whether a particular ioctl should be supported or not. Usually based on capabilities, sometimes because certain ioctls allow 'NOP' operations that allow you to test for their presence.

Quoted 2 lines

Of course, drivers are not always consistent here, but that's a separate problem.

Any "hint" code that would try to do some NOP operations may fail. One of the reasons is that such hint is not documented. Yet, I don't officially support such "hint" methods at the API.

Quoted 6 lines

Quoted 3 lines

Interestingly enough, V4L2 core (v4l2_ioctl() in v4l2-dev.c) does return ENOTTY *right now* when the IOCTL handler is not defined. Have we heard about this up to now? :-)

No, but that's because all drivers have an ioctl handler :-) So you never see ENOTTY.

Well, a V4L1 call now returns -ENOTTY, with the current behaviour.

Btw, there are two drivers returning -ENOTTY, when the device got disconnected (or firmware were not uploaded).

The truth is that the current API specs for return code is bogus.

Quoted 10 lines

Quoted 10 lines

As you mention, switching to ENOTTY in general would change the ABI which would potentially break applications. Can this be handled in a way or another? My understanding is that not many applications would rely on EINVAL
telling an IOCTL isn't implemented. GStreamer v4l2src might be one in its attempt to figure out what kind of image sizes the device supports. Fixing this would be a very small change.

Quoted 7 lines

Quoted 2 lines

In short, I think it would be beneficial to switch to ENOTTY in the long run even if it causes some momentary pain.

I would like that as well, but the V4L2 Specification explicitly mentions EINVAL as the error code if an ioctl is not supported. It has done so since it was created. You cannot just change that. And closed source programs may very well rely on this.

The V4L2 spec needs to be fixed with respect to error codes. Driver authors are much more creative than DocBook authors ;) There are a lot of return codes used by the drivers whose API spec doesn't mention (and, on this subject, the same applies to the DVB API). What I've seen is that:
- Sometimes, a core return code is returned. One of the important examples is the ENOSPC error returned when the usb core refuses to stream when the USB bus reached 80% of the available bandwidth. There's a patch floating around that would allow to override the 80% hard limit, via sysfs. So, if properly documented, an userspace application could give a hint that the user needs to either use a different bus or try to change the hard limit;
- For every ioctl, it presents its own "private" list of error codes. If someone wants to add a new code (for example, standardizing ENOTTY or ENOSPC), all affected ioctl's will need to be touched. This is hard to maintain;
- Drivers are not compliant with error codes.

The right thing to do is to create a separate chapter for error codes, based on errno(3) man page, where we document all error codes that should be used by the drivers. Then, at the ioctl pages, link to the common chapter and, only when needed, document special cases where an error code for that specific ioctl has some special meaning.

I ran a script here to check how many different error codes are used inside drivers/media:

$ find drivers/media -type f -name '*.[ch]'  >files
$ grep define `find . -name errno*.h`|perl -ne 'print "$1\n" if (/\#define\s+(E[^\s]+)/)'|sort|uniq >errors
$ for i in `cat errors`; do COUNT=$(git grep -c $i `cat files`|wc -l); if [ "$COUNT" != "0" ]; then echo $i $COUNT; fi; done

The result is that we're using 53 different types of errors, but the API specs documents only 17 of them. Those are the currently used errors at drivers/media:

ERROR CODE |NUMBER OF *.c/*.h FILES USING IT
---------------|--------------------------------

E2BIG           1
EACCES          8
EAGAIN          66
EBADF           1
EBADFD          1
EBADR           2
EBADRQC         2
EBUSY           149
ECHILD          1
ECONNRESET      25
EDEADLK         1
EDOM            1
EEXIST          3
EFAULT          230
EFBIG           1
EILSEQ          8
EINIT           2
EINPROGRESS     6
EINTR           21
EINVAL          501
EIO             305
EMFILE          1
ENFILE          7
ENOBUFS         7
ENODATA         4
ENODEV          270
ENOENT          46
ENOIOCTLCMD     31
ENOMEM          359
ENOSPC          13
ENOSR           7
ENOSYS          15
ENOTSUP         3
ENOTSUPP        3
ENOTTY          5
ENXIO           26
EOPNOTSUPP      19
EOVERFLOW       14
EPERM           47
EPIPE           12
EPROTO          11
ERANGE          25
EREMOTE         80
EREMOTEIO       80
ERESTART        32
ERESTARTSYS     32
ESHUTDOWN       27
ESPIPE          3
ETIME           53
ETIMEDOUT       37
EUSERS          2
EWOULDBLOCK     14
EXDEV           1

I suspect that we'll need to both fix some drivers, and the API, as I bet that the same error conditions are reported differently on different drivers.

Quoted 1 line

I don't think changing such an important return value is acceptable.

As I said, the current API is bogus with respect to error codes. Of course, we need to do take care to avoid userspace applications breakage, but we can't use the excuse that it is there for a long time as a reason for not fixing it.

Quoted 9 lines

Em 27-06-2011 09:17, Hans Verkuil escreveu:

Quoted 7 lines

While we don't have an enum capability, in many cases you can deduce whether a particular ioctl should be supported or not. Usually based on capabilities, sometimes because certain ioctls allow 'NOP' operations that allow you to test for their presence.

Of course, drivers are not always consistent here, but that's a separate problem.

Quoted 3 lines

Any "hint" code that would try to do some NOP operations may fail. One of the reasons is that such hint is not documented. Yet, I don't officially support such "hint" methods at the API.

The point is that the spec can easily be improved to make such 'NOP' operations explicit, or to require that if a capability is present, then the corresponding ioctl(s) must also be present. Things like that are easy to verify as well with v4l2-compliance.

Quoted 4 lines

Btw, there are two drivers returning -ENOTTY, when the device got disconnected (or firmware were not uploaded).

The truth is that the current API specs for return code is bogus.

Bogus in what way? It's been documented very clearly for years. We may not like that design decision (I certainly don't like it), but someone clearly thought about it at the time.

Quoted 4 lines

The right thing to do is to create a separate chapter for error codes, based on errno(3) man page, where we document all error codes that should be used by the drivers. Then, at the ioctl pages, link to the common chapter and, only when needed, document special cases where an error code for that specific ioctl has some special meaning.

Great, I've no problem with that. But this particular error code you want to change is actually implemented *consistently* in all drivers. There is no confusion, no ambiguity, and it is according to the spec.

Quoted 70 lines

I ran a script here to check how many different error codes are used inside drivers/media:

$ find drivers/media -type f -name '*.[ch]'  >files
$ grep define `find . -name errno*.h`|perl -ne 'print "$1\n" if (/\#define\s+(E[^\s]+)/)'|sort|uniq >errors
$ for i in `cat errors`; do COUNT=$(git grep -c $i `cat files`|wc -l); if [ "$COUNT" != "0" ]; then echo $i $COUNT; fi; done

The result is that we're using 53 different types of errors, but the API specs documents only 17 of them. Those are the currently used errors at drivers/media:

ERROR CODE |NUMBER OF *.c/*.h FILES USING IT
---------------|--------------------------------

E2BIG           1
EACCES          8
EAGAIN          66
EBADF           1
EBADFD          1
EBADR           2
EBADRQC         2
EBUSY           149
ECHILD          1
ECONNRESET      25
EDEADLK         1
EDOM            1
EEXIST          3
EFAULT          230
EFBIG           1
EILSEQ          8
EINIT           2
EINPROGRESS     6
EINTR           21
EINVAL          501
EIO             305
EMFILE          1
ENFILE          7
ENOBUFS         7
ENODATA         4
ENODEV          270
ENOENT          46
ENOIOCTLCMD     31
ENOMEM          359
ENOSPC          13
ENOSR           7
ENOSYS          15
ENOTSUP         3
ENOTSUPP        3
ENOTTY          5
ENXIO           26
EOPNOTSUPP      19
EOVERFLOW       14
EPERM           47
EPIPE           12
EPROTO          11
ERANGE          25
EREMOTE         80
EREMOTEIO       80
ERESTART        32
ERESTARTSYS     32
ESHUTDOWN       27
ESPIPE          3
ETIME           53
ETIMEDOUT       37
EUSERS          2
EWOULDBLOCK     14
EXDEV           1

I suspect that we'll need to both fix some drivers, and the API, as I bet that the same error conditions are reported differently on different drivers.

Quoted 1 line

I don't think changing such an important return value is acceptable.

Quoted 3 lines

As I said, the current API is bogus with respect to error codes. Of course, we need to do take care to avoid userspace applications breakage, but we can't use the excuse that it is there for a long time as a reason for not fixing it.

The fact that many drivers use error codes creatively doesn't give us an excuse to just change the one error code that is actually used everywhere according to the spec! That's faulty logic.

Quoted 4 lines

The point is that the spec can easily be improved to make such 'NOP' operations explicit, or to require that if a capability is present, then the corresponding ioctl(s) must also be present. Things like that are easy to verify as well with v4l2-compliance.

We currently have more than 64 ioctl's. Adding a capability bit for each doesn't seem the right thing to do. Ok, some could be grouped, but, even so, there are drivers that implement the VIDIOC_G, but doesn't implement the corresponding VIDIO_S. So, I think we don't have enough available bits for doing that.

Quoted 9 lines

Quoted 4 lines

The point is that the spec can easily be improved to make such 'NOP' operations explicit, or to require that if a capability is present, then the corresponding ioctl(s) must also be present. Things like that are easy to verify as well with v4l2-compliance.

We currently have more than 64 ioctl's. Adding a capability bit for each doesn't seem the right thing to do. Ok, some could be grouped, but, even so, there are drivers that implement the VIDIOC_G, but doesn't implement the corresponding VIDIO_S. So, I think we don't have enough available bits for doing that.

It shouldn't be too hard to do an ioctl command that returns a le_bitmask with the ioctl command number as an index (0 to 91, currently), and the bit set for each command that has the corresponding v4l2_ioctl_ops member filled for the device. That would be an obvious way to query the operations, but I don't know if it's useful.

Em 27-06-2011 11:56, Hans Verkuil escreveu:

Quoted 10 lines · On Monday, June 27, 2011 15:54:11 Mauro Carvalho Chehab wrote:

Quoted 9 lines

Em 27-06-2011 09:17, Hans Verkuil escreveu:

Quoted 7 lines

While we don't have an enum capability, in many cases you can deduce whether a particular ioctl should be supported or not. Usually based on capabilities, sometimes because certain ioctls allow 'NOP' operations that allow you to test for their presence.

Of course, drivers are not always consistent here, but that's a separate problem.

Quoted 8 lines

Quoted 3 lines

Any "hint" code that would try to do some NOP operations may fail. One of the reasons is that such hint is not documented. Yet, I don't officially support such "hint" methods at the API.

The point is that the spec can easily be improved to make such 'NOP' operations explicit, or to require that if a capability is present, then the corresponding ioctl(s) must also be present. Things like that are easy to verify as well with v4l2-compliance.

We currently have more than 64 ioctl's. Adding a capability bit for each doesn't seem the right thing to do. Ok, some could be grouped, but, even so, there are drivers that implement the VIDIOC_G, but doesn't implement the corresponding VIDIO_S. So, I think we don't have enough available bits for doing that.

Quoted 8 lines

Quoted 4 lines

Btw, there are two drivers returning -ENOTTY, when the device got disconnected (or firmware were not uploaded).

The truth is that the current API specs for return code is bogus.

Bogus in what way? It's been documented very clearly for years. We may not like that design decision (I certainly don't like it), but someone clearly thought about it at the time.

Bogus in the sense that drivers don't follow them, as they're returning undocumented values. Any application strictly following it will have troubles.

Quoted 8 lines

Quoted 4 lines

The right thing to do is to create a separate chapter for error codes, based on errno(3) man page, where we document all error codes that should be used by the drivers. Then, at the ioctl pages, link to the common chapter and, only when needed, document special cases where an error code for that specific ioctl has some special meaning.

Great, I've no problem with that. But this particular error code you want to change is actually implemented *consistently* in all drivers. There is no confusion, no ambiguity, and it is according to the spec.

As I said, from userspace perspective, it is not consistent to assume that EINVAL means not implemented. For sure at VIDIOC_S_foo, this is not consistent. Even on some GET types of ioctl, like for example [1][2], there are other reasons for an EINVAL return.

[1] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-cropcap.html
[2] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-audio.html

The only way to make it consistent is to use different return codes for "invalid parameters" and for "unsupported ioctl".

Quoted 3 lines

The fact that many drivers use error codes creatively doesn't give us an excuse to just change the one error code that is actually used everywhere according to the spec! That's faulty logic.

The fix that it is needed is to provide a consistent way for an userspace application to know for sure when an ioctl is not supported. It can be done on a simple way of just returning a different error code for it, or with complex mechanisms like adding a per-ioctl flag and some hint logics based on NOP.

The V4L2 is complex enough for us to add more complexity with hints and cap flags.

Quoted 20 lines

Em 27-06-2011 11:56, Hans Verkuil escreveu:

Quoted 10 lines · On Monday, June 27, 2011 15:54:11 Mauro Carvalho Chehab wrote:

Quoted 9 lines

Em 27-06-2011 09:17, Hans Verkuil escreveu:

Quoted 7 lines

While we don't have an enum capability, in many cases you can deduce whether a particular ioctl should be supported or not. Usually based on capabilities, sometimes because certain ioctls allow 'NOP' operations that allow you to test for their presence.

Of course, drivers are not always consistent here, but that's a separate problem.

Quoted 8 lines

Quoted 3 lines

Any "hint" code that would try to do some NOP operations may fail. One of the reasons is that such hint is not documented. Yet, I don't officially support such "hint" methods at the API.

The point is that the spec can easily be improved to make such 'NOP' operations explicit, or to require that if a capability is present, then the corresponding ioctl(s) must also be present. Things like that are easy to verify as well with v4l2-compliance.

Quoted 4 lines

We currently have more than 64 ioctl's. Adding a capability bit for each doesn't seem the right thing to do. Ok, some could be grouped, but, even so, there are drivers that implement the VIDIOC_G, but doesn't implement the corresponding VIDIO_S. So, I think we don't have enough available bits for doing that.

No, that's not what I meant.

Whether or not ioctls are implemented can in many cases be deduced from the QUERYCAP capabilities: e.g. if V4L2_CAP_STREAMING is set, then the buffer I/O ioctls have to be there.

For other ioctls the test whether they are implemented
is also often straightforward: e.g. if VIDIOC_G_INPUT returns -EINVAL, then that can only mean that it isn't implemented, and neither are ENUM_INPUT and S_INPUT.

In cases where only the G variant is implemented, there we need to tighten the spec and require that these ioctls are properly implemented: you either implement all of the ENUM/G/TRY/S ioctls or none.

Currently ENUM_FRAMESIZES/INTERVALS is one set of ioctls where this is very ambiguous. For most of the others it is pretty straightforward.

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

Btw, there are two drivers returning -ENOTTY, when the device got disconnected (or firmware were not uploaded).

The truth is that the current API specs for return code is bogus.

Bogus in what way? It's been documented very clearly for years. We may not like that design decision (I certainly don't like it), but someone clearly thought about it at the time.

Quoted 2 lines

Bogus in the sense that drivers don't follow them, as they're returning undocumented values. Any application strictly following it will have troubles.

I suspect that in most cases the drivers are fairly reasonable, but the spec wasn't updated with the new error codes.

Quoted 13 lines

Quoted 8 lines

Quoted 4 lines

The right thing to do is to create a separate chapter for error codes, based on errno(3) man page, where we document all error codes that should be used by the drivers. Then, at the ioctl pages, link to the common chapter and, only when needed, document special cases where an error code for that specific ioctl has some special meaning.

Great, I've no problem with that. But this particular error code you want to change is actually implemented *consistently* in all drivers. There is no confusion, no ambiguity, and it is according to the spec.

As I said, from userspace perspective, it is not consistent to assume that EINVAL means not implemented. For sure at VIDIOC_S_foo, this is not consistent. Even on some GET types of ioctl, like for example [1][2], there are other reasons for an EINVAL return.

Quoted 5 lines

[1] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-cropcap.html
[2] http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-audio.html

The only way to make it consistent is to use different return codes for "invalid parameters" and for "unsupported ioctl".

No, what we do is perfectly consistent: i.e. we always return EINVAL when an ioctl is not supported. That's what 'consistent' means. Whether that is the *right* error code is something else.

But right now our API and our documentation is perfectly consistent and has been for years.

<snip>

Quoted 8 lines

Quoted 3 lines

The fact that many drivers use error codes creatively doesn't give us an excuse to just change the one error code that is actually used everywhere according to the spec! That's faulty logic.

The fix that it is needed is to provide a consistent way for an userspace application to know for sure when an ioctl is not supported. It can be done on a simple way of just returning a different error code for it, or with complex mechanisms like adding a per-ioctl flag and some hint logics based on NOP.

It's a 'fix' that I fear may break applications because drivers suddenly change their behavior. You can't just ignore that. You also can't analyze applications, since closed source apps may also use it, and we obviously have no control over those.

Quoted 1 line

The V4L2 is complex enough for us to add more complexity with hints and cap flags.

Actually, all it needs for the most part is that current implicit rules are made
explicit: if a certain querycap flag is set, then a corresponding set of ioctls must be implemented. If I can call the GET ioctl, then the ENUM and SET (and TRY) must also be implemented. Sensible rules at any time, they just need to be made explicit.

If you want to change it to ENOTTY, then go right ahead. But you can explain it to our customers when their app suddenly breaks for some hardware.

I don't get it. This is really not a problem. We rarely, if ever, get complaints about it, and there is a ton of other much more important stuff that needs to be done.

Regards,

Hans

No, what we do is perfectly consistent: i.e. we always return EINVAL when an ioctl is not supported. That's what 'consistent' means. Whether that is the *right* error code is something else.

Quoted 4 lines · Hans Verkuil

No, what we do is perfectly consistent: i.e. we always return EINVAL when an ioctl is not supported. That's what 'consistent' means. Whether that is the *right* error code is something else.

You don't even understand the problem.

The problem is two-fold:
(a) it's the _wrong_ error code (this part you seem to get)
but also
(b) you ALSO return -EINVAL for ioctl's you support!

How har dis (b) to understand? The fact is, -EINVAL does not mean "I don't support that ioctl". It _should_ mean "I _do_ support that ioctl, but you passed me bogus arguments for that ioctl that I cannot handle".

And right now, for the v4l crowd, -EINVAL means "random error code that _could_ be because the ioctl doesn't exist, but it could also be because the ioctl _does_ exist but didn't like it's value".

See the problem?

The correct error code for "I don't understand this ioctl" is ENOTTY. The naming may be odd, but you should think of that error value as a "unrecognized ioctl number, you're feeding me random numbers that I don't understand and I assume for historical reasons that you tried to do some tty operation on me".

I don't understand why the v4l people have such a hard time getting this. This has been going on for years.

I would suggest that somebody just switch around the EINVAL in __video_do_ioctl() to -ENOTTY (and change the ENOIOCTLCMD translation to also make it ENOTTY instead of EINVAL) and just try to see if anything breaks.

Maybe things actually break, and we'd have to undo it for compatibility reasons (and perhaps add a comment about the program that is so flaky that it needs a EINVAL return from the ioctl), but I really do not understand people like you who seem to argue against doing the right thing without even _trying_ it.

Saying that your API is "consistent" is clearly bullshit. Your API is _wrong_. It's not consistent. Returning EINVAL can mean two different things, there's no way to tell which case it was - that's not "consistent" by any stretch of the imagination.

Linus

Of course, since vfs_ioctl does the same thing this is clearly isn't some isolated V4L thing. Pot. Kettle.

It was my understanding that we shouldn't break the userspace API. This breaks the userspace API. If everyone else says it's fine to break the userspace API this time, then who am I to object?

(In fact, the _correct_ thing to do would probably be to just do

   #define ENOIOCTLCMD ENOTTY

and get rid of any translation - just giving ENOTTY a more appropriate name and less chance for confusion)

There may be applications out there that really break when they get ENOTTY instead of EINVAL. But most cases that check for errors from ioctl's tend to just say "did this succeed or not" rather than "did this return EINVAL". That's *doubly* true since the error code has been ambiguous, so checking for the exact error code has always been pretty pointless.

Quoted 6 lines

(In fact, the correct thing to do would probably be to just do

   #define ENOIOCTLCMD ENOTTY

and get rid of any translation - just giving ENOTTY a more appropriate name and less chance for confusion)

Some code uses the two to separate 'the driver specific helper code doesn't handle this' and 'does handle this'. In that situation you take away the ability of a driver to override a midlayer ioctl with -ENOTTY to say "I don't support this even if most people do"

Quoted 4 lines

It was my understanding that we shouldn't break the userspace API. This breaks the userspace API. If everyone else says it's fine to break the userspace API this time, then who am I to object?

We are indeed never supposed to break userspace.

But that does not mean that we cannot fix bugs. It only means exactly what it says: we cannot break user space.

If we fix a bug, and it turns out that user space actually depends on that bug, then we need to revert the fix. But it never means "you can never make any changes at all to interfaces". It only means "you cannot make any changes that break applications".

There may be applications out there that really break when they get ENOTTY instead of EINVAL. But most cases that check for errors from ioctl's tend to just say "did this succeed or not" rather than "did this return EINVAL". That's *doubly* true since the error code has been ambiguous, so checking for the exact error code has always been pretty pointless.

So the common use of the error code tends to be for things like strerror(), and then it would be a real improvement to show "Inappropriate ioctl for device" when the device doesn't support that ioctl, wouldn't it?

But yes, let's try to fix it, and if it turns out that that breaks something, we must simply revert and probably add a comment to the source code ("EINVAL is wrong, it should be ENOTTY, but xyzzy only works with EINVAL").

The right thing to do is to create a separate chapter for error codes, based on errno(3) man page [snip]

Quoted 3 lines

The right thing to do is to create a separate chapter for error codes, based on errno(3) man page [snip]

IMO, the IEEE Std 1003.1 is a better source for errno definitions than the errno(3) manpage:

http://pubs.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_03.html