2001-02-27 12:38:28

by Per Erik Stendahl

[permalink] [raw]
Subject: Bug in cdrom_ioctl?

Hi.

In linux-2.4.2/drivers/cdrom/cdrom.c:cdrom_ioctl() branches
CDROM_SET_OPTIONS and CDROM_CLEAR_OPTIONS both return like this:

return cdi->options;

If cdi->options is non-zero, the ioctl() calls returns non-zero.
My ioctl(2) manpage says that a successful ioctl() should return
zero. Now I dont know which is at fault here - the cdrom.c code or
the manpage. :-) Could somebody enlighten me?

/Per Erik Stendahl


2001-02-27 13:11:38

by Richard B. Johnson

[permalink] [raw]
Subject: Re: Bug in cdrom_ioctl?

On Tue, 27 Feb 2001, Per Erik Stendahl wrote:

> Hi.
>
> In linux-2.4.2/drivers/cdrom/cdrom.c:cdrom_ioctl() branches
> CDROM_SET_OPTIONS and CDROM_CLEAR_OPTIONS both return like this:
>
> return cdi->options;
>
> If cdi->options is non-zero, the ioctl() calls returns non-zero.
> My ioctl(2) manpage says that a successful ioctl() should return
> zero. Now I dont know which is at fault here - the cdrom.c code or
> the manpage. :-) Could somebody enlighten me?
>
> /Per Erik Stendahl
> -

Specifically, (at the API) upon an error -1 (nothing else) is to be
returned and 'errno' set appropriately. The results of a successful ioctl()
operation is supposed to have been returned in the parameter list (via
pointer). So, you have found a design bug. I wonder how much stuff
gets broken if this gets fixed?

I suggest you just fix it and see what breaks. Maybe sombody's
CD writer will break, but a patch will quickly be made by the
maintainer(s).


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.


2001-02-27 14:46:03

by Per Erik Stendahl

[permalink] [raw]
Subject: RE: Bug in cdrom_ioctl?


> > In linux-2.4.2/drivers/cdrom/cdrom.c:cdrom_ioctl() branches
> > CDROM_SET_OPTIONS and CDROM_CLEAR_OPTIONS both return like this:
> >
> > return cdi->options;
> >
> > If cdi->options is non-zero, the ioctl() calls returns non-zero.
> > My ioctl(2) manpage says that a successful ioctl() should return
> > zero. Now I dont know which is at fault here - the cdrom.c code or
> > the manpage. :-) Could somebody enlighten me?
> >
> > /Per Erik Stendahl
> > -
>
> Specifically, (at the API) upon an error -1 (nothing else) is to be
> returned and 'errno' set appropriately. The results of a
> successful ioctl()
> operation is supposed to have been returned in the parameter list (via
> pointer). So, you have found a design bug. I wonder how much stuff
> gets broken if this gets fixed?

I looked around a bit more in cdrom_ioctl(). There are more cases
where data gets passed back in the return code. If the official
ioctl() policy is to only pass success/fail status in the return
code then it would require some work to fix cdrom_ioctl (breaking
a number of apps in the process :-).

> I suggest you just fix it and see what breaks. Maybe sombody's
> CD writer will break, but a patch will quickly be made by the
> maintainer(s).

For now I did the quickest thing and check ioctl() for -1 instead
of 0.

Is there any good documentation on ioctl calls somewhere?

/Per Erik Stendahl

2001-02-27 16:49:54

by Richard B. Johnson

[permalink] [raw]
Subject: RE: Bug in cdrom_ioctl?

On Tue, 27 Feb 2001, Per Erik Stendahl wrote:

>
> > > In linux-2.4.2/drivers/cdrom/cdrom.c:cdrom_ioctl() branches
> > > CDROM_SET_OPTIONS and CDROM_CLEAR_OPTIONS both return like this:
> > >
> > > return cdi->options;
> > >
> > > If cdi->options is non-zero, the ioctl() calls returns non-zero.
> > > My ioctl(2) manpage says that a successful ioctl() should return
> > > zero. Now I dont know which is at fault here - the cdrom.c code or
> > > the manpage. :-) Could somebody enlighten me?
> > >
> > > /Per Erik Stendahl
> > > -
> >
> > Specifically, (at the API) upon an error -1 (nothing else) is to be
> > returned and 'errno' set appropriately. The results of a
> > successful ioctl()
> > operation is supposed to have been returned in the parameter list (via
> > pointer). So, you have found a design bug. I wonder how much stuff
> > gets broken if this gets fixed?
>
> I looked around a bit more in cdrom_ioctl(). There are more cases
> where data gets passed back in the return code. If the official
> ioctl() policy is to only pass success/fail status in the return
> code then it would require some work to fix cdrom_ioctl (breaking
> a number of apps in the process :-).
>
> > I suggest you just fix it and see what breaks. Maybe somebody's
> > CD writer will break, but a patch will quickly be made by the
> > maintainer(s).
>
> For now I did the quickest thing and check ioctl() for -1 instead
> of 0.
>
> Is there any good documentation on ioctl calls somewhere?
>
> /Per Erik Stendahl
>

Not that I've seen. There may be a C89 spec. Somebody was checking
for POSIX conformance in the kernel API. I don't know if anybody
is doing that anymore.

Looking at various documentation, I see things like "usually -1
indicates error...". This probably means that "anything goes",
which is absurd, but probable. At one time you could get bludgeoned
for writing an ioctl() that returned anything but 0 and -1. Now,
maybe anything goes.

Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.