Re: [PATCH] hw/scsi/megasas:Clean up some redundant code fix Clang warnings

[Peter Maydell]

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.

thanks
-- PMM