> -----Original Message----- > From: Ranbir Singh <rsi...@ventanamicro.com> > Sent: Friday, July 14, 2023 12:47 AM > To: devel@edk2.groups.io; rsi...@ventanamicro.com > Cc: Wu, Hao A <hao.a...@intel.com>; Ni, Ray <ray...@intel.com> > Subject: [PATCH v3 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix > UNUSED_VALUE Coverity issue > > From: Ranbir Singh <ranbir.sin...@dell.com> > > The return value stored in Status after call to SetDriveParameters is not made > of any use thereafter and hence it remains as UNUSED. > > Add error check as is done after calls to SetDeviceTransferMode. > > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Ray Ni <ray...@intel.com> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204 > Signed-off-by: Ranbir Singh <ranbir.sin...@dell.com> > Signed-off-by: Ranbir Singh <rsi...@ventanamicro.com> > --- > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > index 75403886e44a..af022139cf02 100644 > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > @@ -2549,13 +2549,21 @@ DetectAndConfigIdeDevice ( > // > > if (DeviceType == EfiIdeHarddisk) { > > // > > - // Init driver parameters > > + // Init drive parameters > > // > > DriveParameters.Sector = (UINT8)((ATA5_IDENTIFY_DATA > *)(&Buffer.AtaData))->sectors_per_track; > > DriveParameters.Heads = (UINT8)(((ATA5_IDENTIFY_DATA > *)(&Buffer.AtaData))->heads - 1); > > DriveParameters.MultipleSector = (UINT8)((ATA5_IDENTIFY_DATA > *)(&Buffer.AtaData))->multi_sector_cmd_max_sct_cnt; > > > > Status = SetDriveParameters (Instance, IdeChannel, IdeDevice, > &DriveParameters, NULL); > > + > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_WARN, "Set Drive Parameters Fail, Status = %r\n", > + Status)); > > + // > > + // Ignore warning and proceed normally > > + // > > + Status = EFI_SUCCESS;
To please both sides, how about: 1. Remove the 'Status' assignment of the return value from SetDriveParameters() Based on my findings (https://edk2.groups.io/g/devel/message/106844), the successful execution of SetDriveParameters() is not mandatory for initializing IDE mode hard disk device. 2. Add DEBUG_WARN level debug message within SetDriveParameters() function In function SetDriveParameters, for the 2 calls of AtaNonDataCommandIn (one for the INITIALIZE DEVICE PARAMETERS command and the other for SET MULTIPLE MODE command), if the return status is not EFI_SUCCESS, add debug message to display the information. Best Regards, Hao Wu > > + } > > } > > > > // > > -- > 2.34.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106930): https://edk2.groups.io/g/devel/message/106930 Mute This Topic: https://groups.io/mt/100124817/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
