On Sat, Jul 07, 2012 at 09:11:27AM +0200, Hans de Goede wrote:
> Hi,
>
> On 07/06/2012 07:51 PM, Greg Kroah-Hartman wrote:
> >On Wed, Jul 04, 2012 at 09:18:03AM +0200, Hans de Goede wrote:
> >>+static void snoop_urb_data(struct urb *urb, unsigned len)
> >>+{
> >>+ int i, size;
> >>+
> >>+ if (!usbfs_snoop)
> >>+ return;
> >>+
> >>+ if (urb->num_sgs == 0) {
> >>+ print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_NONE, 32, 1,
> >>+ urb->transfer_buffer, len, 1);
> >>+ return;
> >>+ }
> >>+
> >>+ for (i = 0; i < urb->num_sgs && len; i++) {
> >>+ size = (len > USB_SG_SIZE) ? USB_SG_SIZE : len;
> >>+ print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_NONE, 32, 1,
> >>+ sg_virt(&urb->sg[i]), size, 1);
> >>+ len -= size;
> >>+ }
> >>+}
> >
> >Minor cleanup in the future, can't this be merged with snoop_urb() that
> >way you don't have to do the logic checking in the places you call this
> >function instead of snoop_urb()? That would make it a bit simpler for
> >the "normal" code path, right?
>
> Alan already made the same remark :) The problem is that snoop_urb is
> also used for logging the synchronous urb ioctls, where there is no urb.
> So making snoop_urb generic would require it gaining a struct urb *
> argument while keeping its unsigned char *data and int data_len
> arguments, and add an if (urb) ... else in there, so this way seems
> better.
Ok, fair enough. If I get annoyed I'll go see if I can figure out how
to simplify it later, as I think I'm the only one that uses it :)
Actually, I'll probably just move it to use the tracepoint
infrastructure, as that's the proper thing to do in the end.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
