[haiku-commits] Re: r40787 - haiku/trunk/src/system/kernel/fs
- From: Ingo Weinhold <ingo_weinhold@xxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Thu, 03 Mar 2011 11:38:38 +0100
On 2011-03-03 at 10:44:51 [+0100], Jérôme Duval <korli@xxxxxxxxxxxxxxxx>
wrote:
> 2011/3/2 Axel Dörfler <axeld@xxxxxxxxxxxxxxxx>:
> > korli@xxxxxxxxxxxxxxxx wrote:
> >> +++ haiku/trunk/src/system/kernel/fs/fd.cpp 2011-03-02 18:58:14 UTC
> >> (rev 40787)
> >> @@ -518,8 +518,11 @@
> >> if (descriptor->ops->fd_ioctl)
> >> status = descriptor->ops->fd_ioctl(descriptor, op, buffer,
> >> length);
> >> else
> >> - status = B_NOT_SUPPORTED;
> >> + status = B_DEV_INVALID_IOCTL;
> >
> > Not sure this should be replaced, too. At least I think it's not so
> > nice to lose the expressiveness of our error codes because of some
> > poorly documented POSIX functions.
>
> Well I wasn't sure too. These error codes look the same for me, some
> are a bit more specific to the hook like B_DEV_INVALID_IOCTL.
> B_NOT_SUPPORTED is more generic.
> We can well assume the same meaning for both.
>
> >
> >> + if (status == B_DEV_INVALID_IOCTL)
> >> + status = ENOTTY;
> >
> > Isn't that a bit too central? Where is the difference to returning
> > ENOTTY directly this way?
>
> Well I asked, and Ingo suggested to keep this mapping in the kernel
> because ioctl() can be called kernel side too.
Yes, though I thought that would only be done for the two ioctls you
mentioned. Admittedly I hadn't looked at the ioctl() specification before.
Apparently ENOTTY is the right error when the file isn't a STREAMS device
(i.e. that's the one to be returned by "normal" file systems) ENODEV when
the file is a STREAMS device, but generally doesn't support ioctl(), and
EINVAL, if the particular ioctl isn't supported.
> > The POSIX definition of ioctl() is rather useless, I'm afraid, but also
> > states EINVAL for this error case. And since we don't differentiate
> > between ioctl/fcntl on that level, maybe we should move the error code
> > to libroot instead?
I don't see what you mean that we don't differentiate between ioctl() and
fcntl(). The FS hook for fcntl() is set_flags(). The only fuzziness is where
it comes to non-blocking I/O, since that can be set via both and I believe
we don't keep the descriptor's flag in sync when done via ioctl().
I would suggest to consequently use the error codes POSIX specifies for the
respective cases. That would leave B_DEV_INVALID_IOCTL unused. IOW:
* no FS ioctl() hook -> ENOTTY
* devfs, no device ioctl() hook -> ENODEV or ENOTTY (depending on character
vs. block device?)
* in a "normal" FS ioctl() hook: ENOTTY, if not supported
* in a device ioctl() hooks: EINVAL, if not supported
CU, Ingo
Other related posts:
