lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 19 Dec 2012 07:40:52 -0800
From:	Matthew Dharm <mdharm-usb@...-eyed-alien.net>
To:	Sebastian Andrzej Siewior <sebastian@...akpoint.cc>
Cc:	"Fangxiaozhi (Franko)" <fangxiaozhi@...wei.com>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Xueguiying (Zihan)" <zihan.xue@...wei.com>,
	"Linlei (Lei Lin)" <lei.lin@...wei.com>,
	"greg@...ah.com" <greg@...ah.com>,
	"Yili (Neil)" <neil.yi@...wei.com>,
	"Wangyuhua (Roger, Credit)" <wangyuhua@...wei.com>,
	Huqiao <huqiao36@...wei.com>, "balbi@...com" <balbi@...com>
Subject: Re: [PATCH 2/2]linux-usb:optimize to match the Huawei USB storage
 devices and support new switch command

On Wed, Dec 19, 2012 at 12:34 AM, Sebastian Andrzej Siewior
<sebastian@...akpoint.cc> wrote:
> On Wed, Dec 19, 2012 at 03:13:32AM +0000, Fangxiaozhi (Franko) wrote:
>> > And shouldn't you read something from the us->recv_bulk_pipe after
>> > that?
>>       Well, because our device will re-connect to switch the ports if it receives the command.
>>       So it is not necessary to read the response of the command.
>
> Hmm. I guess this for Matthew / Greg to decide, I don't insist on anything.
> Maybe a comment would be nice because now it looks, atleast to me, that
> something is missing.

I think an unusual situation like that deserves a comment that
explains that the device is about to disconnect / reconnect, so
reading status is not necessary.

I am also concerned about the error of using &bcbw instead of bcbw.  I
doubt this code would have worked with that typo in place.  How was
this patch tested?

Also, the dongles_pid function is really just a different
implementation of the unusual_devs.h table.  I think that it is much
easier for people to add new entries to the table, rather than edit
your code, when new dongles are released.  BUT, your code includes
many more PIDs than the table did.  Again, how was this tested for the
new PIDs covered?  At a minimum, some comment in dongles_pid is
required to highlight this area of code for possible future expansion
as new devices are released.

Matt

-- 
Matthew Dharm
Maintainer, USB Mass Storage driver for Linux
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists