[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/scsi/megasas:Clean up some redundant code fix Clang warni
From: |
Laurent Vivier |
Subject: |
Re: [PATCH] hw/scsi/megasas:Clean up some redundant code fix Clang warnings |
Date: |
Tue, 10 Mar 2020 16:04:31 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1 |
Le 10/03/2020 à 14:46, Peter Maydell a écrit :
> On Tue, 10 Mar 2020 at 13:10, Chen Qun <address@hidden> wrote:
>>
>> Here are some redundant statements, we can clean them up.
>> Clang static code analyzer show warning:
>> hw/scsi/megasas.c:1175:32: warning: Value stored to 'max_ld_disks' during
>> its initialization is never read
>> uint32_t num_ld_disks = 0, max_ld_disks = s->fw_luns;
>> ^~~~~~~~~~~~ ~~~~~~~~~~
>> hw/scsi/megasas.c:1183:9: warning: Value stored to 'max_ld_disks' is never
>> read
>> max_ld_disks = 0;
>> ^ ~
>>
>> Reported-by: Euler Robot <address@hidden>
>> Signed-off-by: Chen Qun <address@hidden>
>> ---
>> Cc: Paolo Bonzini <address@hidden>
>> Cc: Fam Zheng <address@hidden>
>> Cc: Hannes Reinecke <address@hidden>
>> Cc: address@hidden
>> ---
>> hw/scsi/megasas.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
>> index af18c88b65..3f982e1d3b 100644
>> --- a/hw/scsi/megasas.c
>> +++ b/hw/scsi/megasas.c
>> @@ -1172,7 +1172,7 @@ static int megasas_dcmd_ld_list_query(MegasasState *s,
>> MegasasCmd *cmd)
>> uint16_t flags;
>> struct mfi_ld_targetid_list info;
>> size_t dcmd_size = sizeof(info), resid;
>> - uint32_t num_ld_disks = 0, max_ld_disks = s->fw_luns;
>> + uint32_t num_ld_disks = 0, max_ld_disks;
>> BusChild *kid;
>>
>> /* mbox0 contains flags */
>> @@ -1180,7 +1180,6 @@ static int megasas_dcmd_ld_list_query(MegasasState *s,
>> MegasasCmd *cmd)
>> trace_megasas_dcmd_ld_list_query(cmd->index, flags);
>> if (flags != MR_LD_QUERY_TYPE_ALL &&
>> flags != MR_LD_QUERY_TYPE_EXPOSED_TO_HOST) {
>> - max_ld_disks = 0;
>> }
>
> This doesn't look right -- your change removes the only statement
> in the body of this "if". I think you need to examine what the
> function is trying to do with the test it is doing on these flags
> in order to identify what the right change is... Probably this
> means going back to the h/w spec to identify the correct behaviour
> overall.
Moreover this "if" is the only user of MR_LD_QUERY_TYPE_ALL and
MR_LD_QUERY_TYPE_EXPOSED_TO_HOST.
Thanks,
Laurent