On 11/24/2016 06:59 AM, fx IWATA NOBUO wrote:
> Hello,
>> This doesn't look like a simple change to rename and reuse an unused
>> function. This patch does lot more and is changing the user interface.
>> Looks like instead of taking an integer value for device lookup, you
>> are changing it to char *.
>> Any reason why you have to change the user interface to go to char
>> *busid?
>> I would like to see a good explanation why this user interface change
>> is necessary.
> I can't figure out why the second argument was int because it was
> unused.

I also cannot figure out this but using busid here looks much more
reasonable to me.

> usbip_get_device() is a stub-side (usb-host) function.
> As you know, only port number in vhci-side can be int.
> Others are bus-id.
> Furthermore, the unused code searches list node position. It doesn't
> make sense.


> Here, this patch set needs to find bound device with bus-id in
> stub-side.

stub-side or vudc side. In case of vudc the busid is set to usbip-vudc.%d

> I may add explanation above to change log.
> Or I can change new function name like usbip_find_device() and leave the
> unused function once.
> Please, let me know how should I do.

In my humble opinion the code itself is good. The problem is commit
message. Current commit massage has nothing to do with patch content and
should be totally changed.

Please elaborate that this function is unused, write that using indexes
on a list is generally not a good idea and so on.

Best regards,
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to