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. > Agree. > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
