[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 5/6] hw/scsi/megasas: Silent GCC9 duplicated-cond warning
From: |
Richard Henderson |
Subject: |
Re: [PATCH 5/6] hw/scsi/megasas: Silent GCC9 duplicated-cond warning |
Date: |
Tue, 17 Dec 2019 18:03:53 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
On 12/17/19 7:34 AM, Philippe Mathieu-Daudé wrote:
> GCC9 is confused when building with CFLAG -O3:
>
> hw/scsi/megasas.c: In function ‘megasas_scsi_realize’:
> hw/scsi/megasas.c:2387:26: error: duplicated ‘if’ condition
> [-Werror=duplicated-cond]
> 2387 | } else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
> hw/scsi/megasas.c:2385:19: note: previously used here
> 2385 | if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
> cc1: all warnings being treated as errors
>
> When this device was introduced in commit e8f943c3bcc, the author
> cared about modularity, using a definition for the firmware limit.
> If we modify the limit, the code is valid. Add a check if the
> definition got modified to a bigger limit.
>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> Cc: Hannes Reinecke <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Fam Zheng <address@hidden>
> Cc: address@hidden
> ---
> hw/scsi/megasas.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index de9bd20887..ece1601b66 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2382,7 +2382,8 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
> **errp)
> if (!s->hba_serial) {
> s->hba_serial = g_strdup(MEGASAS_HBA_SERIAL);
> }
> - if (s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
> + if (MEGASAS_MAX_SGE > 128
> + && s->fw_sge >= MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE) {
> s->fw_sge = MEGASAS_MAX_SGE - MFI_PASS_FRAME_SIZE;
> } else if (s->fw_sge >= 128 - MFI_PASS_FRAME_SIZE) {
> s->fw_sge = 128 - MFI_PASS_FRAME_SIZE;
>
I'm not keen on this. It looks to me like the raw 128 case should be removed
-- surely that's the point of the symbolic constant. But I'll defer if a
maintainer disagrees.
r~