Hello, > 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. OK. I will improve the commit message. Thank you, n.iwata // > -----Original Message----- > From: Krzysztof Opasiak [mailto:k.opasiak@xxxxxxxxxxx] > Sent: Friday, December 02, 2016 4:57 AM > To: fx IWATA NOBUO; shuah.kh@xxxxxxxxxxx; valentina.manea.m@xxxxxxxxx; > gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx > Cc: fx MICHIMURA TADAO; Shuah Khan > Subject: Re: [PATCH v13 02/10] usbip: exporting devices: modifications to > host side libraries > > > > 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
