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é <phi...@redhat.com> > --- > Cc: Hannes Reinecke <h...@suse.com> > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: Fam Zheng <f...@euphon.net> > Cc: qemu-bl...@nongnu.org > --- > 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~