Hi Simon,
On Thu, Aug 31, 2017 at 5:52 AM, Simon Glass sjg@chromium.org wrote:
Hi Suneel,
On 15 August 2017 at 11:06, Suneel Garapati suneelglinux@gmail.com wrote:
Hi Simon,
On Sun, Aug 13, 2017 at 2:37 PM, Simon Glass sjg@chromium.org wrote:
Hi Suneel,
On 10 August 2017 at 23:53, Suneel Garapati suneelglinux@gmail.com wrote:
usb tree and info commands may cause crash otherwise
Signed-off-by: Suneel Garapati suneelglinux@gmail.com
cmd/usb.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Thank you for the patch - it certainly looks like a bug. Can you please expand the commit message a little? E.g. you have UCLASS_USB_EMUL below.
I will change the description
diff --git a/cmd/usb.c b/cmd/usb.c index 992d414..81e1a7b 100644 --- a/cmd/usb.c +++ b/cmd/usb.c @@ -415,7 +415,8 @@ static void usb_show_tree_graph(struct usb_device *dev, char *pre) udev = dev_get_parent_priv(child);
/* Ignore emulators, we only want real devices */
if (device_get_uclass_id(child) != UCLASS_USB_EMUL) {
if (device_get_uclass_id(child) !=(UCLASS_USB_EMUL | UCLASS_BLK)) {This seems odd to me. Do you mean to check that the child uclass is neither USB_EMUL nor BLK?
Would it be possible to check that the parent is UCLASS_USB? That seems like a better condition to determine whether the child has USB parent data.
It is possible to check parent uclass but would that ever fail?
How would it fail? The only device that does not have a parent is the root device, and we know it cannot be that since it will be UCLASS_ROOT.
I assume, block device under usb storage device will always have parent as usb class.
Yes that sounds right.
Also, tree is called on only usb class devices. Maybe I am missing something.
Maybe, but I think it is more robust to check for the thing you want than the things you don't. If someone adds a new thing that can be a child then this code will need updating.
I will add a check on parent uclass for USB in v1.
Regards, Suneel
Regards, Suneel
usb_show_tree_graph(udev, pre); pre[index] = 0; }@@ -605,7 +606,8 @@ static void usb_show_info(struct usb_device *udev) for (device_find_first_child(udev->dev, &child); child; device_find_next_child(&child)) {
if (device_active(child)) {
if (device_active(child) &&(device_get_uclass_id(child) != UCLASS_BLK)) { udev = dev_get_parent_priv(child); usb_show_info(udev); }-- 2.7.4
Regards, Simon