cosa.h ioctl numbers

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.

@@ -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/ |

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.

@@ -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/ |

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

Andreas.

Andreas Schwab, SuSE Labs, [email protected]
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."

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/ |

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, [email protected]
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."

Andreas Schwab wrote:
: Jan Kasprzak <[email protected]> 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/ |

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

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, [email protected]
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."

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

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.

@@ -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/ |