Hi Bart,
> On Dec 20, 2017, at 10:25 AM, Bart Van Assche <bart.vanassche@xxxxxxx> wrote:
>
> 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”?
>
Sure. The intention of patch was to cleanup and make sure there is memory allocated
on the stack for name.
>> +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",
> ...
> };
>
I will drop this patch from the current submission.
>> --- 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?
>
Sure. will drop this change
> Thanks,
>
> Bart.
Thanks,
- Himanshu
