On Tue, 29 Jan 2008, Boaz Harrosh wrote:
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -462,18 +462,24 @@ static int usb_stor_bulk_transfer_sglist(struct us_data *us, unsigned int pipe,
> * Common used function. Transfer a complete command
> * via usb_stor_bulk_transfer_sglist() above. Set cmnd resid
> */
> -int usb_stor_bulk_srb(struct us_data* us, unsigned int pipe,
> - struct scsi_cmnd* srb)
> +int usb_stor_bulk_srb_length(struct us_data* us, unsigned int pipe,
> + struct scsi_cmnd* srb, unsigned length)
> {
> unsigned int partial;
> int result = usb_stor_bulk_transfer_sglist(us, pipe, scsi_sglist(srb),
> - scsi_sg_count(srb), scsi_bufflen(srb),
> + scsi_sg_count(srb), length,
> &partial);
>
> scsi_set_resid(srb, scsi_bufflen(srb) - partial);
> return result;
> }
>
> +int usb_stor_bulk_srb(struct us_data* us, unsigned int pipe,
> + struct scsi_cmnd* srb)
> +{
> + return usb_stor_bulk_srb_length(us, pipe, srb, scsi_bufflen(srb));
> +}
> +
I don't like this patch very much. Why add another layer of
indirection when the two subroutines do hardly any work? Leave
usb_stor_bulk_srb() the way it was, and add usb_stor_bulk_srb_length()
as a separate routine that simply calls usb_stor_bulk_transfer_sglist()
and scsi_set_resid().
BTW, the standard coding style calls for a blank line after the list of
local variables at the start of a function or block.
Alan Stern
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
