[PATCH] cosa.h ioctl numbers

[PATCH] cosa.h ioctl numbers

[Jan Kasprzak]

	The following patch reverts the changes in ioctl() numbers
for COSA WAN card, makink the ioctl numbers the same as in 2.4, and thus
preserving the binary compatibility with user-space utils.

Signed-off-by: Jan "Yenya" Kasprzak <kas@fi.muni.cz>

--- linux-2.6.10-rc2/drivers/net/wan/cosa.h.orig	2004-12-02 13:34:24.142501564 +0100
+++ linux-2.6.10-rc2/drivers/net/wan/cosa.h	2004-12-02 13:35:59.770705004 +0100
@@ -76,10 +76,10 @@
 #define COSAIOSTRT	_IOW('C',0xf1, int)
 
 /* Read the block from the device memory */
-#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download)
+#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download *)
 
 /* Write the block to the device memory (i.e. download the microcode) */
-#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download)
+#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download *)
 
 /* Read the device type (one of "srp", "cosa", and "cosa8" for now) */
 #define COSAIORTYPE	_IOR('C',0xf3, char *)

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
> Whatever the Java applications and desktop dances may lead to, Unix will <
> still be pushing the packets around for a quite a while.      --Rob Pike <

Re: [PATCH] cosa.h ioctl numbers

[Arnd Bergmann]

[-- Attachment #1: Type: text/plain, Size: 880 bytes --]

On Dunnersdag 02 Dezember 2004 13:44, Jan Kasprzak wrote:
> 	The following patch reverts the changes in ioctl() numbers
> for COSA WAN card, makink the ioctl numbers the same as in 2.4, and thus
> preserving the binary compatibility with user-space utils.

>  /* Read the block from the device memory */
> -#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download)
> +#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download *)
>  
>  /* Write the block to the device memory (i.e. download the microcode) */
> -#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download)
> +#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download *)

Isn't that rather misleading? I suppose the real argument is 
'struct cosa_download', so you should have some kind of comment there, 
e.g.

#define COSAIODOWNLD _IOW('C',0xf2, long) /* actually struct cosa_download */

	Arnd <><

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] cosa.h ioctl numbers

[Jan Kasprzak]

Arnd Bergmann wrote:
: >  /* Write the block to the device memory (i.e. download the microcode) */
: > -#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download)
: > +#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download *)
: 
: Isn't that rather misleading? I suppose the real argument is 
: 'struct cosa_download', so you should have some kind of comment there, 
: e.g.
: 
: #define COSAIODOWNLD _IOW('C',0xf2, long) /* actually struct cosa_download */

	Well, the third argument of ioctl(2) is of type
struct cosa_download *.

	OK, second try with comments added.

Signed-off-by: Jan "Yenya" Kasprzak <kas@fi.muni.cz>

--- linux-2.6.10-rc2/drivers/net/wan/cosa.h.orig	2004-12-02 13:34:24.142501564 +0100
+++ linux-2.6.10-rc2/drivers/net/wan/cosa.h	2004-12-02 14:09:23.000860524 +0100
@@ -76,10 +76,16 @@
 #define COSAIOSTRT	_IOW('C',0xf1, int)
 
 /* Read the block from the device memory */
-#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download)
+#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download *)
+	/* actually the struct cosa_download itself; this is to keep
+	 * the ioctl number same as in 2.4 in order to keep the user-space
+	 * utils compatible. */
 
 /* Write the block to the device memory (i.e. download the microcode) */
-#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download)
+#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download *)
+	/* actually the struct cosa_download itself; this is to keep
+	 * the ioctl number same as in 2.4 in order to keep the user-space
+	 * utils compatible. */
 
 /* Read the device type (one of "srp", "cosa", and "cosa8" for now) */
 #define COSAIORTYPE	_IOR('C',0xf3, char *)


-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
> Whatever the Java applications and desktop dances may lead to, Unix will <
> still be pushing the packets around for a quite a while.      --Rob Pike <

Re: [PATCH] cosa.h ioctl numbers

[Andreas Schwab]

Jan Kasprzak <kas@fi.muni.cz> writes:

> Arnd Bergmann wrote:
> : >  /* Write the block to the device memory (i.e. download the microcode) */
> : > -#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download)
> : > +#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download *)
> : 
> : Isn't that rather misleading? I suppose the real argument is 
> : 'struct cosa_download', so you should have some kind of comment there, 
> : e.g.
> : 
> : #define COSAIODOWNLD _IOW('C',0xf2, long) /* actually struct cosa_download */
>
> 	Well, the third argument of ioctl(2) is of type
> struct cosa_download *.
>
> 	OK, second try with comments added.

If you want real compatibility you should use size_t, which is what 2.4 is
effectively using.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] cosa.h ioctl numbers

[Jan Kasprzak]

Andreas Schwab wrote:
: If you want real compatibility you should use size_t, which is what 2.4 is
: effectively using.
: 
	I assume that sizeof(struct .. *) == sizeof(size_t) on i386.
COSA is an old ISA-based device (out of production now), and I don't think
it will be ever used in non-i386 machine. In fact it is the first time
someone tried to use it on 2.6 kernel (and thus I had to fix the driver :-).

-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
> Whatever the Java applications and desktop dances may lead to, Unix will <
> still be pushing the packets around for a quite a while.      --Rob Pike <

Re: [PATCH] cosa.h ioctl numbers

[Andreas Schwab]

Jan Kasprzak <kas@fi.muni.cz> writes:

> Andreas Schwab wrote:
> : If you want real compatibility you should use size_t, which is what 2.4 is
> : effectively using.
> : 
> 	I assume that sizeof(struct .. *) == sizeof(size_t) on i386.

This has nothing to do with this, but everything to do with
sizeof(sizeof(foo)) == sizeof(size_t).  And COSAIODOWNLD does not expect a
pointer to a pointer but a pointer to struct cosa_download, which means
that _IOW('C',0xf2,struct cosa_download *) would be completely wrong
anyway.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] cosa.h ioctl numbers

[Linus Torvalds]

On Thu, 2 Dec 2004, Andreas Schwab wrote:
> 
> This has nothing to do with this, but everything to do with
> sizeof(sizeof(foo)) == sizeof(size_t).  And COSAIODOWNLD does not expect a
> pointer to a pointer but a pointer to struct cosa_download, which means
> that _IOW('C',0xf2,struct cosa_download *) would be completely wrong
> anyway.

We have similar "broken due to historical reasons" in other places. 
There's no good way to fix them up, and it doesn't really matter _what_ 
fake type you use, as long as it happens to be the same size as the 
original broken type on all architectures where it matters.

We've done different things in the past depending on exactly what the
numbers have been. Sometimes "size_t" (to keep pointers happy on both
32-bit and 64-bit architectures), sometimes "int" or "u32" (when somebody
had used an array _member_ instead of the array, or just had the size
explicitly haldcoded as an integer number, and then "size_t" just made it
"sizeof(int)" etc).

Sounds like it doesn't much matter in this case, any of the above would 
work.

		Linus

Re: [PATCH] cosa.h ioctl numbers

[Jan Kasprzak]

Andreas Schwab wrote:
: Jan Kasprzak <kas@fi.muni.cz> writes:
: 
: > Andreas Schwab wrote:
: > : If you want real compatibility you should use size_t, which is what 2.4 is
: > : effectively using.
: > : 
: > 	I assume that sizeof(struct .. *) == sizeof(size_t) on i386.
: 
: This has nothing to do with this, but everything to do with
: sizeof(sizeof(foo)) == sizeof(size_t).  And COSAIODOWNLD does not expect a
: pointer to a pointer but a pointer to struct cosa_download, which means
: that _IOW('C',0xf2,struct cosa_download *) would be completely wrong
: anyway.

	I do not understand. The _IOW() macro just uses sizeof(_third_argument)
both on 2.4 and 2.6. And nothing else than sizeof() is done with the third
argument of _IOW(). So it does not matter what you put into the third
argument anyway, provided that you make sure the ABI (i.e. the type
and layout of the last argument to ioctl()) remains the same. The third
argument to _IOW() is just a (rather weak) helper which helps you to detect
the unwanted ABI change.

	Yes, I made a (small) mistake when writing cosa.h during the
Linux 2.1 development cycle, but since nobody cared and I am pretty
sure I did not change the layout of struct cosa_download at all since
Linux 2.1, I would rather have the 2.6 ioctl numbers the same
as in 2.1-2.4.
 
-Yenya

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
> Whatever the Java applications and desktop dances may lead to, Unix will <
> still be pushing the packets around for a quite a while.      --Rob Pike <

Re: [PATCH] cosa.h ioctl numbers

[Andreas Schwab]

Jan Kasprzak <kas@fi.muni.cz> writes:

> 	I do not understand. The _IOW() macro just uses sizeof(_third_argument)
> both on 2.4 and 2.6.

Yes, and 2.4 uses sizeof in the third argument, thus size_t is the most
natural replacement.

> I would rather have the 2.6 ioctl numbers the same as in 2.1-2.4.

You get that when you use size_t for the type, which also gives some hint
that the old definition was an unfortunate mistake and might prevent other
people from "fixing" it again.  Putting a pointer here just adds to the
confusion and should be avoided, IMHO.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] cosa.h ioctl numbers

[Jan Kasprzak]

Linus Torvalds wrote:
: On Thu, 2 Dec 2004, Andreas Schwab wrote:
: > This has nothing to do with this, but everything to do with
: > sizeof(sizeof(foo)) == sizeof(size_t).  And COSAIODOWNLD does not expect a
: > pointer to a pointer but a pointer to struct cosa_download, which means
: > that _IOW('C',0xf2,struct cosa_download *) would be completely wrong
: > anyway.
: 
: We have similar "broken due to historical reasons" in other places. 
: There's no good way to fix them up, and it doesn't really matter _what_ 
: fake type you use, as long as it happens to be the same size as the 
: original broken type on all architectures where it matters.
[...]
: Sounds like it doesn't much matter in this case, any of the above would 
: work.
[...]
	OK, then please apply the attached patch:

Description: Make COSA ioctl numbers compatible with previous kernels.

Signed-off-by: Jan "Yenya" Kasprzak <kas@fi.muni.cz>

--- linux-2.6.10-rc2/drivers/net/wan/cosa.h.orig	2004-12-02 13:34:24.142501564 +0100
+++ linux-2.6.10-rc2/drivers/net/wan/cosa.h	2004-12-02 14:09:23.000860524 +0100
@@ -76,10 +76,16 @@
 #define COSAIOSTRT	_IOW('C',0xf1, int)
 
 /* Read the block from the device memory */
-#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download)
+#define COSAIORMEM	_IOWR('C',0xf2, struct cosa_download *)
+	/* actually the struct cosa_download itself; this is to keep
+	 * the ioctl number same as in 2.4 in order to keep the user-space
+	 * utils compatible. */
 
 /* Write the block to the device memory (i.e. download the microcode) */
-#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download)
+#define COSAIODOWNLD	_IOW('C',0xf2, struct cosa_download *)
+	/* actually the struct cosa_download itself; this is to keep
+	 * the ioctl number same as in 2.4 in order to keep the user-space
+	 * utils compatible. */
 
 /* Read the device type (one of "srp", "cosa", and "cosa8" for now) */
 #define COSAIORTYPE	_IOR('C',0xf3, char *)

-- 
| Jan "Yenya" Kasprzak  <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839      Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/   Czech Linux Homepage: http://www.linux.cz/ |
> Whatever the Java applications and desktop dances may lead to, Unix will <
> still be pushing the packets around for a quite a while.      --Rob Pike <