Takashi Sakamoto wrote:
(Apr 04 2014 18:31), Clemens Ladisch wrote:
Takashi Sakamoto wrote:
+/* each field should be in big endian */ +struct snd_efw_transaction {
- uint32_t length;
- uint32_t version;
- uint32_t seqnum;
- uint32_t category;
- uint32_t command;
- uint32_t status;
- uint32_t params[0];
+};
uint32_t is not guaranteed to be available in a userspace header. Add <linux/types.h>, and use __u32.
Can I request the reason?
Here I force users to include <stdint.h> and use types defined in ISO C99. I think this way is standard. (but not standard between kernel/user-land?)
Headers should always be self-contained, i.e., they must include any other headers they need.
As for the ISO C types, Linus Torvalds wrote: | On Mon, 29 Nov 2004, Paul Mackerras wrote: | > | > uint32_t is defined to be exactly 32 bits wide, so where's the problem | > in using it instead of __u32 in the headers that describe the | > user/kernel interface? (Ditto for uint{8,16,64}_t, of course. | | The kernel uses u8/u16/u32 because: | | - the kernel should not depend on, or pollute user-space naming. | YOU MUST NOT USE "uint32_t" when that may not be defined, and | user-space rules for when it is defined are arcane and totally | arbitrary. | | - since the kernel cannot use those types for anything that is | visible to user space anyway, there has to be alternate names. | The tradition is to prepend two underscores, so the kernel would | have to use "__uint32_t" etc for its header files. | | - at that point, there's no longer any valid argument that it's a | "standard type" (it ain't), and I personally find it a lot more | readable to just use the types that the kernel has always used: | __u8/__u16/__u32. For stuff that is only used for the kernel, | the shorter "u8/u16/u32" versions may be used. | | In short: having the kernel use the same names as user space is ACTIVELY | BAD, exactly because those names have standards-defined visibility, which | means that the kernel _cannot_ use them in all places anyway. So don't | even _try_. http://yarchive.net/comp/linux/kernel_headers.html
+static long +hwdep_write(struct snd_hwdep *hwdep, const char __user *data, long count,
loff_t *offset)+{
- ...
- if (count < sizeof(struct snd_efw_transaction))
return -EINVAL;The size must not be larger than allowed for a single write transaction. This would eventually be caught by the firewire core, but checking it here would avoid allocating lots of memory.
I investigate the length of all commands and realize the maximum bytes is 304 (0x130) for isochronous channel mapping. With leaving some space for this value, I think 0x140 is better for this purpose.
IEEE-1394 defines the maximum payload for asynchronous data packets (see table 6-4); for S100/S200/S400/S800, it's 512/1024/2048/4096 bytes.
Fireworks devices always are S400; just use the maximum of 0x200 bytes. (Anything less risks incompatibilities with an old kernel when some mixer application wants to use a new command introduced in some new firmware version.)
Regards, Clemens