Re: [PATCH 5/6] usb: usbip tool: Fix refresh_imported_device_list()
Hi Krzysztof, On Mon, Mar 27, 2017 at 04:31:42PM +0200, Krzysztof Opasiak wrote: > > > On 03/27/2017 07:25 AM, Yuyang Du wrote: > >On Mon, Mar 27, 2017 at 09:07:50AM +0200, Krzysztof Opasiak wrote: > >> > >>As now we have multiple controllers I would be more than happy if we > >>could fix functions like this one to take a controller as a > >>parameter and invoke commands on it instead hardcoding loops like > >>this one with some unclear conditions like if (i > 0). > > > >You mean something like this? > > > >int parse_controller_status(int controller) { > > > > if (controller > 0) > > ... > > > > ... > > > >} > > > > Rather: > > int parse_status(controller *ctrl) > { > stat = get_status(ctrl->status_path); > > ... > } I hope I know how to get the status_path easily, but I don't. So I'll post another version with this unaddressed. Please make it in detail and I'll make another version. Thanks, Yuyang -- 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
Re: [PATCH 5/6] usb: usbip tool: Fix refresh_imported_device_list()
On 03/27/2017 07:25 AM, Yuyang Du wrote: On Mon, Mar 27, 2017 at 09:07:50AM +0200, Krzysztof Opasiak wrote: As now we have multiple controllers I would be more than happy if we could fix functions like this one to take a controller as a parameter and invoke commands on it instead hardcoding loops like this one with some unclear conditions like if (i > 0). You mean something like this? int parse_controller_status(int controller) { if (controller > 0) ... ... } Rather: int parse_status(controller *ctrl) { stat = get_status(ctrl->status_path); ... } Best regards, -- Krzysztof Opasiak Samsung R 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
Re: [PATCH 5/6] usb: usbip tool: Fix refresh_imported_device_list()
On Mon, Mar 27, 2017 at 09:07:50AM +0200, Krzysztof Opasiak wrote: > > As now we have multiple controllers I would be more than happy if we > could fix functions like this one to take a controller as a > parameter and invoke commands on it instead hardcoding loops like > this one with some unclear conditions like if (i > 0). You mean something like this? int parse_controller_status(int controller) { if (controller > 0) ... ... } Thanks, Yuyang -- 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
Re: [PATCH 5/6] usb: usbip tool: Fix refresh_imported_device_list()
+ Shuah Khan On 03/23/2017 03:46 AM, Yuyang Du wrote: The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications to vhci driver") introduced multiple controllers, but the status of the ports are only extracted from the first status file, fix it. Signed-off-by: Yuyang Du--- tools/usb/usbip/libsrc/vhci_driver.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index d335f04..1c0e7f0 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -35,14 +35,11 @@ return NULL; } - - Unrelated (put to separate commit) static int parse_status(const char *value) { int ret = 0; char *c; - The same here. for (int i = 0; i < vhci_driver->nports; i++) memset(_driver->idev[i], 0, sizeof(vhci_driver->idev[i])); @@ -107,18 +104,33 @@ static int parse_status(const char *value) return 0; } +#define MAX_STATUS_NAME 16 + static int refresh_imported_device_list(void) { const char *attr_status; + char status[MAX_STATUS_NAME+1] = "status"; + int i, ret; - attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device, - "status"); - if (!attr_status) { - err("udev_device_get_sysattr_value failed"); - return -1; + for (i = 0; i < vhci_driver->ncontrollers; i++) { + if (i > 0) + snprintf(status, MAX_STATUS_NAME+1, "status.%d", i); s/MAX_STATUS_NAME+1/sizeof(status) + + attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device, + status); + if (!attr_status) { + err("udev_device_get_sysattr_value failed"); + return -1; + } + + dbg("controller %d", i); + + ret = parse_status(attr_status); + if (ret != 0) + return ret; } - return parse_status(attr_status); + return 0; } As now we have multiple controllers I would be more than happy if we could fix functions like this one to take a controller as a parameter and invoke commands on it instead hardcoding loops like this one with some unclear conditions like if (i > 0). Best regards, -- Krzysztof Opasiak Samsung R 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
[PATCH 5/6] usb: usbip tool: Fix refresh_imported_device_list()
The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications to vhci driver") introduced multiple controllers, but the status of the ports are only extracted from the first status file, fix it. Signed-off-by: Yuyang Du--- tools/usb/usbip/libsrc/vhci_driver.c | 30 +- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tools/usb/usbip/libsrc/vhci_driver.c b/tools/usb/usbip/libsrc/vhci_driver.c index d335f04..1c0e7f0 100644 --- a/tools/usb/usbip/libsrc/vhci_driver.c +++ b/tools/usb/usbip/libsrc/vhci_driver.c @@ -35,14 +35,11 @@ return NULL; } - - static int parse_status(const char *value) { int ret = 0; char *c; - for (int i = 0; i < vhci_driver->nports; i++) memset(_driver->idev[i], 0, sizeof(vhci_driver->idev[i])); @@ -107,18 +104,33 @@ static int parse_status(const char *value) return 0; } +#define MAX_STATUS_NAME 16 + static int refresh_imported_device_list(void) { const char *attr_status; + char status[MAX_STATUS_NAME+1] = "status"; + int i, ret; - attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device, - "status"); - if (!attr_status) { - err("udev_device_get_sysattr_value failed"); - return -1; + for (i = 0; i < vhci_driver->ncontrollers; i++) { + if (i > 0) + snprintf(status, MAX_STATUS_NAME+1, "status.%d", i); + + attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device, + status); + if (!attr_status) { + err("udev_device_get_sysattr_value failed"); + return -1; + } + + dbg("controller %d", i); + + ret = parse_status(attr_status); + if (ret != 0) + return ret; } - return parse_status(attr_status); + return 0; } static int get_nports(void) -- 1.9.1 -- 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