Skip to contentSkip to conversation

usb: usbtmc: Add vendor specific/asynchronous read/write ioctls

PATCH New Greg Kroah-Hartman 1 participant 1 message May 18, 2018 USB

lore
Share & tools
lore

Greg Kroah-Hartman

author maintainer
#0 lore raw

PATCH 6/12 usb: usbtmc: Add vendor specific/asynchronous read/write ioctls

Quoted 26 lines · On Fri, May 18, 2018 at 02:48:11PM +0000, [email protected] wrote:

Zitat von Greg KH <[email protected]>:

Quoted 19 lines · Guido Kiener

Review

Quoted 13 lines
(quoted diff) +13 -0
+/*
+ * usbtmc_message->flags:
+ */
+#define USBTMC_FLAG_ASYNC		0x0001
+#define USBTMC_FLAG_APPEND		0x0002
+#define USBTMC_FLAG_IGNORE_TRAILER	0x0004
+
+struct usbtmc_message {
+	void __user *message; /* pointer to header and data */
+	__u64 transfer_size; /* size of bytes to transfer */
+	__u64 transferred; /* size of received/written bytes */
+	__u32 flags; /* bit 0: 0 = synchronous; 1 = asynchronous */
+} __attribute__ ((packed));

Very odd structure. Your userspace pointer is going to be totally out of alignment on 32bit systems running on a 64bit kernel. Why have a separate pointer at all? Why not just put the mesage at the end of this structure directly with something like:

Quoted 3 lines
	__u8 message[0];

?

Quoted 5 lines

Oh yes, I should know that from another project. Indead we did not test it with 32 bit applications on 64 platforms.

When we use your proposal then we just can use #define USBTMC_IOCTL_WRITE _IO(USBTMC_IOC_NR, 13)?

Why 13? Use the structure please, that way you know you always get it right.

thanks,

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html