Hi, Thanks for the reviews!
On 08/22/2012 01:42 AM, Pete Batard wrote: > Thanks for these patches. Just a couple points so far: > > On 2012.08.20 22:10, Hans de Goede wrote: >> + __u32 caps; > > Are stdint.h and the kernel expected to be potentially out of sync with > regards to what constitutes an 32 bit unsigned integer, or could we not > use the much more universal uint32_t there, especially as that's what we > use everywhere else? I would prefer to stick with the __u32, as that is what is in the kernels *userspace* headers, which we need anyways for the _IOWR / _IOR / _IOW macros. Ideally we would not have linux_usbfs.h at all, and instead use "#include <linux/usbdevice_fs.h>", but that will break things if that header is too old, so we're stuck with a private copy of that file... > The rationale would be that even if __u32 is what the kernel prefers, > we're building a user library. > > Also, with regards to the second patch, since we're going to divide by it: > > + bulk_buffer_len = transfer->length; > I'm not seeing much validation being done in fill_bulk_transfer() and > submit_transfer() to ensure that transfer->length is never zero. > Isn't that something we should take some provisions for? Oh, good point. Let me audit the code in question for any divide by zero issues. > Apart from that, these 2 patches look good to me. Ok, I'll take a look into the divide by 0 issues, and then resend all 3 patches taking your comments into account. Regards, Hans ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/ _______________________________________________ libusbx-devel mailing list libusbx-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/libusbx-devel
