Hello,

> > +   rc = get_importable_devices(host, sockfd);
> >     if (rc < 0) {
> >             err("failed to get device list from %s", host);
> > +           close(sockfd);
> >             return -1;
> >     }
> Why not just close before if?

I will move before if.

> After fixing suggestion about placement of close() function you may
> add my:
> 
> Reviewed-by: Krzysztof Opasiak <k.opas...@samsung.com>

Sure.

> In addition I think that this patch should be send separately before
> this whole series.

After this set is re-arranged. I will remind it.
I may include this patch because (it can be said that) this caused by
introducing export request.

Thank you for your help,

n.iwata
//
> -----Original Message-----
> From: Krzysztof Opasiak [mailto:k.opas...@samsung.com]
> Sent: Saturday, December 03, 2016 12:53 AM
> To: fx IWATA NOBUO; valentina.mane...@gmail.com; shuah...@samsung.com;
> gre...@linuxfoundation.org; linux-usb@vger.kernel.org
> Cc: fx MICHIMURA TADAO
> Subject: Re: [PATCH v13 08/10] usbip: exporting devices: change to
> usbip_list.c
> 
> 
> 
> On 11/22/2016 07:48 AM, Nobuo Iwata wrote:
> > Correction to wording inconsistency around import and export in
> > usbip_list.c regarding output title, help and function names.
> >
> > 'exported' was used for devices bound in remote and to be attached
> > with 'import' request. This patch set uses pre-defined 'export'
> > request to connect device.
> >
> > To avoid mixed usage of 'export', 'importable' is used for devices to
> > be attached with 'import' request.
> >
> > The word 'imported' has already been used in output of port operation.
> > It is consistent to this patch.
> >
> 
> After fixing suggestion about placement of close() function you may add
> my:
> 
> Reviewed-by: Krzysztof Opasiak <k.opas...@samsung.com>
> 
> In addition I think that this patch should be send separately before this
> whole series.
> 
> > Signed-off-by: Nobuo Iwata <nobuo.iw...@fujixerox.co.jp>
> > ---
> >  tools/usb/usbip/src/usbip_list.c | 17 +++++++++--------
> >  1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/usb/usbip/src/usbip_list.c
> > b/tools/usb/usbip/src/usbip_list.c
> > index f1b38e8..cf69f31 100644
> > --- a/tools/usb/usbip/src/usbip_list.c
> > +++ b/tools/usb/usbip/src/usbip_list.c
> > @@ -44,7 +44,7 @@
> >  static const char usbip_list_usage_string[] =
> >     "usbip list [-p|--parsable] <args>\n"
> >     "    -p, --parsable         Parsable list format\n"
> > -   "    -r, --remote=<host>    List the exportable USB devices on
> <host>\n"
> > +   "    -r, --remote=<host>    List the importable USB devices on
> <host>\n"
> >     "    -l, --local            List the local USB devices\n";
> >
> >  void usbip_list_usage(void)
> > @@ -52,7 +52,7 @@ void usbip_list_usage(void)
> >     printf("usage: %s", usbip_list_usage_string);  }
> >
> > -static int get_exported_devices(char *host, int sockfd)
> > +static int get_importable_devices(char *host, int sockfd)
> >  {
> >     char product_name[100];
> >     char class_name[100];
> > @@ -82,14 +82,14 @@ static int get_exported_devices(char *host, int
> sockfd)
> >             return -1;
> >     }
> >     PACK_OP_DEVLIST_REPLY(0, &reply);
> > -   dbg("exportable devices: %d\n", reply.ndev);
> > +   dbg("importable devices: %d\n", reply.ndev);
> >
> >     if (reply.ndev == 0) {
> > -           info("no exportable devices found on %s", host);
> > +           info("no importable devices found on %s", host);
> >             return 0;
> >     }
> >
> > -   printf("Exportable USB devices\n");
> > +   printf("Importable USB devices\n");
> >     printf("======================\n");
> >     printf(" - %s\n", host);
> >
> > @@ -134,7 +134,7 @@ static int get_exported_devices(char *host, int
> sockfd)
> >     return 0;
> >  }
> >
> > -static int list_exported_devices(char *host)
> > +static int list_importable_devices(char *host)
> >  {
> >     int rc;
> >     int sockfd;
> > @@ -147,9 +147,10 @@ static int list_exported_devices(char *host)
> >     }
> >     dbg("connected to %s:%s", host, usbip_port_string);
> >
> > -   rc = get_exported_devices(host, sockfd);
> > +   rc = get_importable_devices(host, sockfd);
> >     if (rc < 0) {
> >             err("failed to get device list from %s", host);
> > +           close(sockfd);
> >             return -1;
> >     }
> 
> Why not just close before if?
> 
> 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