Hi Nobuo,
On 12/26/2016 12:08 AM, Nobuo Iwata wrote:
> This patch set implements operations to export and unexport device
> using pre-defined but un-used export and un-export request and reply.
Get rid of the above from the changelog.
>
> This patch modifies export and un-export reply in
> tools/usb/usbip/src/usbip_network.h. The 'returncode' of each response
> is removed. The status is set in op_common.status.
Please don't use file names, instead reference the structures.
As an example this change log would be lot clear as below:
"Add new status codes to network common header struct op_common.
Add corresponding user friendly string for each of the status
codes. Change dbg messages to use new status strings"
There is no need to describe code in the change log.
>
> Following status codes are added for this patch set.
> ST_NO_FREE_POR
> ST_NOT_FOUND
>
> The reason to use op_common.status:
> 1) usbip_net_recv_op_common() hanles as error when the status is other
> than ST_OK.
> 2) The status has a commnet "add more error code".
>
> They become empty struct. Other empty struct, 'op_devlist_request',
> defined.
Does this go with this patch. Doesn't look like the above is accurate.
Don't see op_devlist_request in this patch.
>
> This patch also includes string translation of the status codes.
>
> Signed-off-by: Nobuo Iwata <nobuo.iwata@xxxxxxxxxxxxxxx>
> ---
> tools/usb/usbip/src/usbip_network.c | 26 +++++++++++++++++++++++++-
> tools/usb/usbip/src/usbip_network.h | 8 ++++----
> 2 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/tools/usb/usbip/src/usbip_network.c b/tools/usb/usbip/src/usbip_network.c
> index b4c37e7..069ec54 100644
> --- a/tools/usb/usbip/src/usbip_network.c
> +++ b/tools/usb/usbip/src/usbip_network.c
> @@ -163,6 +163,29 @@ int usbip_net_send_op_common(int sockfd, uint32_t code, uint32_t status)
> return 0;
> }
>
> +struct op_common_error {
> + uint32_t status;
> + const char *str;
> +};
> +
> +struct op_common_error op_common_errors[] = {
> + {ST_NA, "not available"},
> + {ST_NO_FREE_PORT, "no free port"},
> + {ST_DEVICE_NOT_FOUND, "device not found"},
> + {0, NULL}
> +};
> +
> +static const char *op_common_strerror(uint32_t status)
> +{
> + struct op_common_error *err;
> +
> + for (err = op_common_errors; err->str != NULL; err++) {
> + if (err->status == status)
> + return err->str;
> + }
> + return "unknown error";
> +}
This loop is unnecessary. Do a range check on the passed in status
value and index and return the string.
Move the op_common_errors[] and op_common_strerror() usbip_network.h
This can easily be a macro.
Change other places ST_OK handling is done. I see some in usbipd.c
> +
> int usbip_net_recv_op_common(int sockfd, uint16_t *code)
> {
> struct op_common op_common;
> @@ -196,7 +219,8 @@ int usbip_net_recv_op_common(int sockfd, uint16_t *code)
> }
>
> if (op_common.status != ST_OK) {
> - dbg("request failed at peer: %d", op_common.status);
> + dbg("request failed at peer: %s",
> + op_common_strerror(op_common.status));
> goto err;
> }
>
> diff --git a/tools/usb/usbip/src/usbip_network.h b/tools/usb/usbip/src/usbip_network.h
> index c1e875c..86c3ff0 100644
> --- a/tools/usb/usbip/src/usbip_network.h
> +++ b/tools/usb/usbip/src/usbip_network.h
> @@ -27,8 +27,10 @@ struct op_common {
> uint16_t code;
>
> /* add more error code */
codes instead of code
> -#define ST_OK 0x00
> -#define ST_NA 0x01
> +#define ST_OK 0x00
> +#define ST_NA 0x01
> +#define ST_NO_FREE_PORT 0x02
> +#define ST_DEVICE_NOT_FOUND 0x03
Maintain the indentation for the last entry.
> uint32_t status; /* op_code status (for reply) */
>
> } __attribute__((packed));
> @@ -93,7 +95,6 @@ struct op_export_request {
> } __attribute__((packed));
>
> struct op_export_reply {
> - int returncode;
> } __attribute__((packed));
>
Remove the empty structure.
>
> @@ -115,7 +116,6 @@ struct op_unexport_request {
> } __attribute__((packed));
>
> struct op_unexport_reply {
> - int returncode;
> } __attribute__((packed));
Remove the empty structure.
>
> #define PACK_OP_UNEXPORT_REQUEST(pack, request) do {\
>
thanks,
-- Shuah
--
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
