Hello,
> + if (!error)
> + reply.returncode = 0;
> + else
> + reply.returncode = -1;
>
> It looks a little bit ugly to me that we change the value to be unsigned
> and then assign a signed number to it. In addition your compiler is going
> to complain if you use -Wconversion flag.
My mistake.
> > struct op_unexport_reply {
> > - int returncode;
> > + uint32_t returncode;
> > } __attribute__((packed));
*** I will remove 'returncode'.
It's enough by using op_common.status(ST_OK|ST_NA).
There 2 error case for each operation.
Import : not-found, export-error
Export: busy(no free), attach-error
Unexport : not-found, export-error
Existing, import, doesn't carry the reason.
It only uses op_common.status.
Currently, usbip's exit-status doesn't have the reason.
There's a president with no field: op_devlist_request.
Thank you for reviewing,
n.iwata
//
> -----Original Message-----
> From: Krzysztof Opasiak [mailto:k.opasiak@xxxxxxxxxxx]
> Sent: Friday, December 02, 2016 4:11 AM
> To: fx IWATA NOBUO; valentina.manea.m@xxxxxxxxx; shuah.kh@xxxxxxxxxxx;
> gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx
> Cc: fx MICHIMURA TADAO
> Subject: Re: [PATCH v13 01/10] usbip: exporting devices: modifications to
> network header
>
> Hi,
>
> On 11/22/2016 07:48 AM, Nobuo Iwata wrote:
> > Modification to export and un-export response in
> > tools/usb/usbip/src/usbip_network.h. It just changes return code type
> > from int to uint32_t as same as other responses.
> >
> > Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> > ---
> > tools/usb/usbip/src/usbip_network.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/usb/usbip/src/usbip_network.h
> > b/tools/usb/usbip/src/usbip_network.h
> > index c1e875c..e1ca86a 100644
> > --- a/tools/usb/usbip/src/usbip_network.h
> > +++ b/tools/usb/usbip/src/usbip_network.h
> > @@ -93,7 +93,7 @@ struct op_export_request { }
> > __attribute__((packed));
> >
> > struct op_export_reply {
> > - int returncode;
> > + uint32_t returncode;
> > } __attribute__((packed));
> >
> >
> > @@ -115,7 +115,7 @@ struct op_unexport_request { }
> > __attribute__((packed));
> >
> > struct op_unexport_reply {
> > - int returncode;
> > + uint32_t returncode;
> > } __attribute__((packed));
> >
> > #define PACK_OP_UNEXPORT_REQUEST(pack, request) do {\
> >
>
> The field name is returncode but we have no suitable defines with "return
> codes". I ran through USBIP documentation but didn't find any list of allowed
> return codes. Is there any?
>
> You change the value type to unsigned but later you use:
>
> + if (!error)
> + reply.returncode = 0;
> + else
> + reply.returncode = -1;
>
> It looks a little bit ugly to me that we change the value to be unsigned
> and then assign a signed number to it. In addition your compiler is going
> to complain if you use -Wconversion flag.
>
> In my opinion we should have suitable defines for error codes and those
> code should be documented in protocol documentation.
>
> 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
