On Tue, 2017-12-19 at 22:56 -0800, Himanshu Madhani wrote:
> Name pointer for sp describing each command is assigned with stack
> frame's memory. The stack frame could eventually be re-use, where
> name pointer access can get get garbage. This patch designates
> static memory for name pointer to fix this problem.

Which stack memory accesses have been removed by this patch? Sorry but I
haven't found any stack memory access changes in this patch. Additionally,
I haven't found any changes in this patch that look useful to me. Are you
aware that for statements like "str = "unknown"" the compiler allocates
static memory for the string "unknown"?

> +struct sp_name {
> +     uint16_t cmd;
> +     const char *str;
> +};
> +

[ ... ]
 
> +struct sp_name sp_str[] = {
> +     { SPCN_UNKNOWN, "unknown" },
> +     { SPCN_GIDPN, "gidpn" },
> +     { SPCN_GPSC, "gpsc" },
> +     { SPCN_GPNID, "gpnid" },
> +     { SPCN_GPNFT, "gpnft" },
> +     { SPCN_GNNID, "gnnid" },
> +     { SPCN_GFPNID, "gfpnid" },
> +     { SPCN_GFFID, "gffid" },
> +     { SPCN_LOGIN, "login" },
> +     { SPCN_LOGOUT, "logout" },
> +     { SPCN_ADISC, "adisc" },
> +     { SPCN_GNLIST, "gnlist" },
> +     { SPCN_GPDB, "gpdb" },
> +     { SPCN_TMF, "tmf" },
> +     { SPCN_ABORT, "abort" },
> +     { SPCN_NACK, "nack" },
> +     { SPCN_BSG_RPT, "bsg_els_rpt" },
> +     { SPCN_BSG_HST, "bsg_els_hst" },
> +     { SPCN_BSG_CT, "bsg_ct" },
> +     { SPCN_BSG_FX_MGMT, "bsg_fx_mgmt" },
> +     { SPCN_ELS_DCMD, "ELS_DCMD" },
> +     { SPCN_FXDISC, "fxdisc" },
> +     { SPCN_PRLI, "prli" },
> +     { SPCN_NVME_LS, "nvme_ls" },
> +     { SPCN_NVME_CMD, "nvme_cmd" },
> +};

If you want to keep the sp_str[] array after what I wrote above, please
remove the sp_name structure and change sp_str[] into something like the
following:

static const char *sp_str[] = {
        [SPCN_UNKNOWN] = "unknown",
        ...
};

> --- a/drivers/scsi/qla2xxx/qla_mbx.c
> +++ b/drivers/scsi/qla2xxx/qla_mbx.c
> @@ -14,6 +14,7 @@ static struct mb_cmd_name {
>       uint16_t cmd;
>       const char *str;
>  } mb_str[] = {
> +     {0, "unknown mb"},
>       {MBC_GET_PORT_DATABASE,         "GPDB"},
>       {MBC_GET_ID_LIST,               "GIDList"},
>       {MBC_GET_LINK_PRIV_STATS,       "Stats"},
> @@ -24,12 +25,12 @@ static const char *mb_to_str(uint16_t cmd)
>       int i;
>       struct mb_cmd_name *e;
>  
> -     for (i = 0; i < ARRAY_SIZE(mb_str); i++) {
> +     for (i = 1; i < ARRAY_SIZE(mb_str); i++) {
>               e = mb_str + i;
>               if (cmd == e->cmd)
>                       return e->str;
>       }
> -     return "unknown";
> +     return mb_str[0].str;
>  }

Sorry but the above change does not look useful to me in any way. Is this
just code churn?

Thanks,

Bart.

Reply via email to