From: Ranbir Singh <Ranbir.Singh3@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.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
Signed-off-by: Ranbir Singh <rsingh@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;
+ }
}
//
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106919): https://edk2.groups.io/g/devel/message/106919
Mute This Topic: https://groups.io/mt/100124817/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-> -----Original Message-----
> From: Ranbir Singh <rsingh@ventanamicro.com>
> Sent: Friday, July 14, 2023 12:47 AM
> To: devel@edk2.groups.io; rsingh@ventanamicro.com
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v3 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix
> UNUSED_VALUE Coverity issue
>
> From: Ranbir Singh <Ranbir.Singh3@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.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
> Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
> Signed-off-by: Ranbir Singh <rsingh@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> > 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. > Interestingly, my very first patch for this began with this approach only. > 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. > These can be added if desired so. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106935): https://edk2.groups.io/g/devel/message/106935 Mute This Topic: https://groups.io/mt/100124817/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.