Re: [PATCH 12/20] scsi: merge print_opcode_name()

Yoshihiro YUNOMAE
2014-09-05T01:24:45Z

(2014/09/03 19:06), Hannes Reinecke wrote:
> Instead of having two versions of print_opcode_name() we
> should be consolidating them into one version.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/scsi/constants.c | 90 +++++++++++++++++++-----------------------------
> 1 file changed, 35 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
> index 364349c..3aee43b 100644
> --- a/drivers/scsi/constants.c
> +++ b/drivers/scsi/constants.c
> @@ -30,6 +30,11 @@
> #define THIRD_PARTY_COPY_IN 0x84
>
>
> +struct sa_name_list {
> + int cmd;
> + const struct value_name_pair *arr;
> + int arr_sz;
> +};
>
> #ifdef CONFIG_SCSI_CONSTANTS
> static const char * cdb_byte0_names[] = {
> @@ -244,12 +249,6 @@ static const struct value_name_pair variable_length_arr[] = {
> };
> #define VARIABLE_LENGTH_SZ ARRAY_SIZE(variable_length_arr)
>
> -struct sa_name_list {
> - int cmd;
> - const struct value_name_pair *arr;
> - int arr_sz;
> -};
> -
> static struct sa_name_list sa_names_arr[] = {
> {VARIABLE_LENGTH_CMD, variable_length_arr, VARIABLE_LENGTH_SZ},
> {MAINTENANCE_IN, maint_in_arr, MAINT_IN_SZ},
> @@ -267,6 +266,28 @@ static struct sa_name_list sa_names_arr[] = {
> };
> #define SA_NAME_LIST_SZ ARRAY_SIZE(sa_names_arr)

We can delete SA_NAME_LIST_SZ because we define it out of
CONFIG_SCSI_CONSTANTS.

The others look good.

Thanks,
Yoshihiro YUNOMAE

> +#else /* ifndef CONFIG_SCSI_CONSTANTS */
> +static const char * cdb_byte0_names[] = NULL;
> +
> +static struct sa_name_list sa_names_arr[] = {
> + {VARIABLE_LENGTH_CMD, NULL, 0},
> + {MAINTENANCE_IN, NULL, 0},
> + {MAINTENANCE_OUT, NULL, 0},
> + {PERSISTENT_RESERVE_IN, NULL, 0},
> + {PERSISTENT_RESERVE_OUT, NULL, 0},
> + {SERVICE_ACTION_IN_12, NULL, 0},
> + {SERVICE_ACTION_OUT_12, NULL, 0},
> + {SERVICE_ACTION_BIDIRECTIONAL, NULL, 0},
> + {SERVICE_ACTION_IN_16, NULL, 0},
> + {SERVICE_ACTION_OUT_16, NULL, 0},
> + {THIRD_PARTY_COPY_IN, NULL, 0},
> + {THIRD_PARTY_COPY_OUT, NULL, 0},
> + {0, NULL, 0},
> +};
> +#endif /* CONFIG_SCSI_CONSTANTS */
> +#define SA_NAME_LIST_SZ ARRAY_SIZE(sa_names_arr)
> +#define CDB_BYTE0_NAMES_SZ ARRAY_SIZE(cdb_byte0_names)
> +
> static bool scsi_opcode_sa_name(int cmd, int service_action,
> const char **sa_name)
> {
> @@ -316,11 +337,14 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)
>
> if (!scsi_opcode_sa_name(cdb0, sa, &name)) {
> if (cdb0 < 0xc0) {
> - name = cdb_byte0_names[cdb0];
> - if (name)
> - printk("%s", name);
> - else
> - printk("cdb[0]=0x%x (reserved)", cdb0);
> + if (CDB_BYTE0_NAMES_SZ) {
> + name = cdb_byte0_names[cdb0];
> + if (name)
> + printk("%s", name);
> + else
> + printk("cdb[0]=0x%x (reserved)", cdb0);
> + } else
> + printk("cdb[0]=0x%x", cdb0);
> } else
> printk("cdb[0]=0x%x (vendor)", cdb0);
> } else {
> @@ -334,50 +358,6 @@ static void print_opcode_name(unsigned char * cdbp, int cdb_len)
> }
> }
>
> -#else /* ifndef CONFIG_SCSI_CONSTANTS */
> -
> -static void print_opcode_name(unsigned char * cdbp, int cdb_len)
> -{
> - int sa, len, cdb0;
> -
> - cdb0 = cdbp[0];
> - switch(cdb0) {
> - case VARIABLE_LENGTH_CMD:
> - len = scsi_varlen_cdb_length(cdbp);
> - if (len < 10) {
> - printk("short opcode=0x%x command, len=%d "
> - "ext_len=%d", cdb0, len, cdb_len);
> - break;
> - }
> - sa = (cdbp[8] << 8) + cdbp[9];
> - printk("cdb[0]=0x%x, sa=0x%x", cdb0, sa);
> - if (len != cdb_len)
> - printk(", in_cdb_len=%d, ext_len=%d", len, cdb_len);
> - break;
> - case MAINTENANCE_IN:
> - case MAINTENANCE_OUT:
> - case PERSISTENT_RESERVE_IN:
> - case PERSISTENT_RESERVE_OUT:
> - case SERVICE_ACTION_IN_12:
> - case SERVICE_ACTION_OUT_12:
> - case SERVICE_ACTION_BIDIRECTIONAL:
> - case SERVICE_ACTION_IN_16:
> - case SERVICE_ACTION_OUT_16:
> - case THIRD_PARTY_COPY_IN:
> - case THIRD_PARTY_COPY_OUT:
> - sa = cdbp[1] & 0x1f;
> - printk("cdb[0]=0x%x, sa=0x%x", cdb0, sa);
> - break;
> - default:
> - if (cdb0 < 0xc0)
> - printk("cdb[0]=0x%x", cdb0);
> - else
> - printk("cdb[0]=0x%x (vendor)", cdb0);
> - break;
> - }
> -}
> -#endif
> -
> void __scsi_print_command(unsigned char *cdb)
> {
> int k, len;
>

--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@hitachi.com