Re: [PATCH 5/6] usb: usbip tool: Fix refresh_imported_device_list()

2017-03-31 Thread Yuyang Du
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()

2017-03-27 Thread Krzysztof Opasiak



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()

2017-03-27 Thread Yuyang Du
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()

2017-03-27 Thread Krzysztof Opasiak


+ 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()

2017-03-23 Thread Yuyang Du
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