From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.27674.1685919817883414072 for ; Sun, 04 Jun 2023 16:03:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=BTT3Z4gO; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 6A8C161197 for ; Sun, 4 Jun 2023 23:03:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D131EC4339E for ; Sun, 4 Jun 2023 23:03:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685919816; bh=jTedvGV6tcEExTffErBIoSwCSEe8BRb5+/VmtXf2yck=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=BTT3Z4gOwvi1XbTPM/ppcAOCfRQ/lUFqKOO1OR0n3JNoDgl1gCXpQ1LEXpyZ8TUah p2Ml0kJvNdmsT84qlnw6PH002kPnkBu+mZoRJrP9LbfQyNpUZ9J4yOORkvgNI7IbGX cKVQ1a0aGDTZK+lhquy3barlYvbFF4N8agTWQG4beaqlCPp/BlQ8Bq9JkNU4U6Mmnw zSjiEcxg81Exc8LPh0f3y1f1O+JaqU8AslVehzAdAfbO9MNYBKEmL8X0zbwUtdgoKH MOmRXh8Nbvu8VjIUbJcr7GB2VAUygrrXuPqwd0nlOJF8kpXxK+LJCLp1wDs+1TjFce WAv4CFmEJOyBQ== Received: by mail-lf1-f44.google.com with SMTP id 2adb3069b0e04-4f4e71a09a7so5208397e87.1 for ; Sun, 04 Jun 2023 16:03:36 -0700 (PDT) X-Gm-Message-State: AC+VfDxJCbAd89lGzgT4Uqneq6YcbezrHzehKKy/4+nu2B2iKBqbyTjc nZp4aOSu2wWPPifHx+lJrFTZGdv/+MlPvFL1VTM= X-Google-Smtp-Source: ACHHUZ79Bg2ygv6FTAVT8CAz3T8ub/kHcKGDxHxyohjesZocessl3hItx9c0ruRVCGntH5B/+8xhFlPqogjfGbCjGPQ= X-Received: by 2002:a05:6512:402:b0:4f5:bc4e:80f7 with SMTP id u2-20020a056512040200b004f5bc4e80f7mr2935650lfk.14.1685919814785; Sun, 04 Jun 2023 16:03:34 -0700 (PDT) MIME-Version: 1.0 References: <20230602070946.83730-1-rsingh@ventanamicro.com> <20230602070946.83730-2-rsingh@ventanamicro.com> In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 5 Jun 2023 01:03:23 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue To: devel@edk2.groups.io, pedro.falcato@gmail.com Cc: rsingh@ventanamicro.com, Hao A Wu , Ray Ni Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sat, 3 Jun 2023 at 18:04, Pedro Falcato wrote: > > On Fri, Jun 2, 2023 at 8:42=E2=80=AFPM Ranbir Singh wrote: > > > > From: Ranbir Singh > > > > The return value stored in Status after call to SetDriveParameters > > is not made of any use thereafter and hence it remains as UNUSED. > > Assuming, this non-usage is deliberate, the storage in Status can be > > done away with. > > > > Cc: Hao A Wu > > Cc: Ray Ni > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4204 > > Signed-off-by: Ranbir Singh > > --- > > MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c b/MdeModul= ePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > > index 75403886e44a..c6d637afa989 100644 > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c > > @@ -2555,7 +2555,7 @@ DetectAndConfigIdeDevice ( > > DriveParameters.Heads =3D (UINT8)(((ATA5_IDENTIFY_DATA = *)(&Buffer.AtaData))->heads - 1); > > DriveParameters.MultipleSector =3D (UINT8)((ATA5_IDENTIFY_DATA *= )(&Buffer.AtaData))->multi_sector_cmd_max_sct_cnt; > > > > - Status =3D SetDriveParameters (Instance, IdeChannel, IdeDevice, = &DriveParameters, NULL); > > + SetDriveParameters (Instance, IdeChannel, IdeDevice, &DriveParam= eters, NULL); > > I'm /fairly/ sure this is wrong and that you need to use Status. > Yeah, removing the assignment fixes the coverity warning, but now you are hiding a bug instead of fixing it. SetDriveParameters () can apparently fail, and this is being ignored. At the very least, we should emit a diagnostic here in DEBUG mode to log this. E.g., DEBUG ((DEBUG_WARN, "%a: SetDriveParameters () failed - %r\n", __func__, Status));