* [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion
@ 2018-07-30 2:51 Zeng, Star
2018-07-30 5:35 ` Kinney, Michael D
0 siblings, 1 reply; 4+ messages in thread
From: Zeng, Star @ 2018-07-30 2:51 UTC (permalink / raw)
To: edk2-devel@lists.01.org; +Cc: Kinney, Michael D, Zeng, Star
Update the title to include "[staging/FmpDevicePkg-master]".
Thanks,
Star
-----Original Message-----
From: Zeng, Star
Sent: Monday, July 30, 2018 10:50 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion
Some static tool reports "DeviceLibLowestSupportedVersion" is used, but is uninitialized. It is false positive reporting based because DeviceLibLowestSupportedVersion will have assigned value after
FmpDeviceGetLowestSupportedVersion() returns SUCCESS.
This patch refines the code to initialize DeviceLibLowestSupportedVersion to DEFAULT_LOWESTSUPPORTEDVERSION before calling FmpDeviceGetLowestSupportedVersion().
It can pass the static tool's check.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
FmpDevicePkg/FmpDxe/FmpDxe.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c index b25a1511c70c..4868f80de2b9 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -229,10 +229,8 @@ GetLowestSupportedVersion (
//
// Check the FmpDeviceLib
//
- Status = FmpDeviceGetLowestSupportedVersion (&DeviceLibLowestSupportedVersion);
- if (EFI_ERROR (Status)) {
- DeviceLibLowestSupportedVersion = DEFAULT_LOWESTSUPPORTEDVERSION;
- }
+ DeviceLibLowestSupportedVersion = DEFAULT_LOWESTSUPPORTEDVERSION;
+ FmpDeviceGetLowestSupportedVersion (&DeviceLibLowestSupportedVersion);
if (DeviceLibLowestSupportedVersion > ReturnLsv) {
ReturnLsv = DeviceLibLowestSupportedVersion;
--
2.7.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion
2018-07-30 2:51 [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion Zeng, Star
@ 2018-07-30 5:35 ` Kinney, Michael D
2018-07-30 5:41 ` Zeng, Star
0 siblings, 1 reply; 4+ messages in thread
From: Kinney, Michael D @ 2018-07-30 5:35 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org, Kinney, Michael D
Star,
I do not think the logic is identical.
The function FmpDeviceGetLowestSupportedVersion() could
modify the value of DeviceLibLowestSupportedVersion and
return an error.
Mike
> -----Original Message-----
> From: Zeng, Star
> Sent: Sunday, July 29, 2018 7:51 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: [staging/FmpDevicePkg-master][PATCH]
> FmpDevicePkg FmpDxe: Initialize
> DeviceLibLowestSupportedVersion
>
> Update the title to include "[staging/FmpDevicePkg-
> master]".
>
> Thanks,
> Star
> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, July 30, 2018 10:50 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [PATCH] FmpDevicePkg FmpDxe: Initialize
> DeviceLibLowestSupportedVersion
>
> Some static tool reports
> "DeviceLibLowestSupportedVersion" is used, but is
> uninitialized. It is false positive reporting based
> because DeviceLibLowestSupportedVersion will have
> assigned value after
> FmpDeviceGetLowestSupportedVersion() returns SUCCESS.
>
> This patch refines the code to initialize
> DeviceLibLowestSupportedVersion to
> DEFAULT_LOWESTSUPPORTEDVERSION before calling
> FmpDeviceGetLowestSupportedVersion().
> It can pass the static tool's check.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> FmpDevicePkg/FmpDxe/FmpDxe.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c index
> b25a1511c70c..4868f80de2b9 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -229,10 +229,8 @@ GetLowestSupportedVersion (
> //
> // Check the FmpDeviceLib
> //
> - Status = FmpDeviceGetLowestSupportedVersion
> (&DeviceLibLowestSupportedVersion);
> - if (EFI_ERROR (Status)) {
> - DeviceLibLowestSupportedVersion =
> DEFAULT_LOWESTSUPPORTEDVERSION;
> - }
> + DeviceLibLowestSupportedVersion =
> DEFAULT_LOWESTSUPPORTEDVERSION;
> + FmpDeviceGetLowestSupportedVersion
> (&DeviceLibLowestSupportedVersion);
>
> if (DeviceLibLowestSupportedVersion > ReturnLsv) {
> ReturnLsv = DeviceLibLowestSupportedVersion;
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion
2018-07-30 5:35 ` Kinney, Michael D
@ 2018-07-30 5:41 ` Zeng, Star
2018-07-30 5:43 ` Kinney, Michael D
0 siblings, 1 reply; 4+ messages in thread
From: Zeng, Star @ 2018-07-30 5:41 UTC (permalink / raw)
To: Kinney, Michael D, edk2-devel@lists.01.org; +Cc: Zeng, Star
Normally, FmpDeviceGetLowestSupportedVersion() should not touch input DeviceLibLowestSupportedVersion when returning an error.
If we think it is legal that DeviceLibLowestSupportedVersion could be updated in an error case by FmpDeviceGetLowestSupportedVersion(), then we could only add new line below before calling FmpDeviceGetLowestSupportedVersion().
DeviceLibLowestSupportedVersion = DEFAULT_LOWESTSUPPORTEDVERSION;
Thanks,
Star
-----Original Message-----
From: Kinney, Michael D
Sent: Monday, July 30, 2018 1:36 PM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion
Star,
I do not think the logic is identical.
The function FmpDeviceGetLowestSupportedVersion() could modify the value of DeviceLibLowestSupportedVersion and return an error.
Mike
> -----Original Message-----
> From: Zeng, Star
> Sent: Sunday, July 29, 2018 7:51 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [staging/FmpDevicePkg-master][PATCH]
> FmpDevicePkg FmpDxe: Initialize
> DeviceLibLowestSupportedVersion
>
> Update the title to include "[staging/FmpDevicePkg- master]".
>
> Thanks,
> Star
> -----Original Message-----
> From: Zeng, Star
> Sent: Monday, July 30, 2018 10:50 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [PATCH] FmpDevicePkg FmpDxe: Initialize
> DeviceLibLowestSupportedVersion
>
> Some static tool reports
> "DeviceLibLowestSupportedVersion" is used, but is uninitialized. It is
> false positive reporting based because DeviceLibLowestSupportedVersion
> will have assigned value after
> FmpDeviceGetLowestSupportedVersion() returns SUCCESS.
>
> This patch refines the code to initialize
> DeviceLibLowestSupportedVersion to DEFAULT_LOWESTSUPPORTEDVERSION
> before calling FmpDeviceGetLowestSupportedVersion().
> It can pass the static tool's check.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> FmpDevicePkg/FmpDxe/FmpDxe.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c index
> b25a1511c70c..4868f80de2b9 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -229,10 +229,8 @@ GetLowestSupportedVersion (
> //
> // Check the FmpDeviceLib
> //
> - Status = FmpDeviceGetLowestSupportedVersion
> (&DeviceLibLowestSupportedVersion);
> - if (EFI_ERROR (Status)) {
> - DeviceLibLowestSupportedVersion =
> DEFAULT_LOWESTSUPPORTEDVERSION;
> - }
> + DeviceLibLowestSupportedVersion =
> DEFAULT_LOWESTSUPPORTEDVERSION;
> + FmpDeviceGetLowestSupportedVersion
> (&DeviceLibLowestSupportedVersion);
>
> if (DeviceLibLowestSupportedVersion > ReturnLsv) {
> ReturnLsv = DeviceLibLowestSupportedVersion;
> --
> 2.7.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion
2018-07-30 5:41 ` Zeng, Star
@ 2018-07-30 5:43 ` Kinney, Michael D
0 siblings, 0 replies; 4+ messages in thread
From: Kinney, Michael D @ 2018-07-30 5:43 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org, Kinney, Michael D
That proposal looks better.
Mike
> -----Original Message-----
> From: Zeng, Star
> Sent: Sunday, July 29, 2018 10:42 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [staging/FmpDevicePkg-master][PATCH]
> FmpDevicePkg FmpDxe: Initialize
> DeviceLibLowestSupportedVersion
>
> Normally, FmpDeviceGetLowestSupportedVersion() should
> not touch input DeviceLibLowestSupportedVersion when
> returning an error.
> If we think it is legal that
> DeviceLibLowestSupportedVersion could be updated in an
> error case by FmpDeviceGetLowestSupportedVersion(),
> then we could only add new line below before calling
> FmpDeviceGetLowestSupportedVersion().
>
> DeviceLibLowestSupportedVersion =
> DEFAULT_LOWESTSUPPORTEDVERSION;
>
>
> Thanks,
> Star
>
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Monday, July 30, 2018 1:36 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-
> devel@lists.01.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: RE: [staging/FmpDevicePkg-master][PATCH]
> FmpDevicePkg FmpDxe: Initialize
> DeviceLibLowestSupportedVersion
>
> Star,
>
> I do not think the logic is identical.
>
> The function FmpDeviceGetLowestSupportedVersion() could
> modify the value of DeviceLibLowestSupportedVersion and
> return an error.
>
> Mike
>
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Sunday, July 29, 2018 7:51 PM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Zeng, Star
> > <star.zeng@intel.com>
> > Subject: [staging/FmpDevicePkg-master][PATCH]
> > FmpDevicePkg FmpDxe: Initialize
> > DeviceLibLowestSupportedVersion
> >
> > Update the title to include "[staging/FmpDevicePkg-
> master]".
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Monday, July 30, 2018 10:50 AM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael
> D
> > <michael.d.kinney@intel.com>
> > Subject: [PATCH] FmpDevicePkg FmpDxe: Initialize
> > DeviceLibLowestSupportedVersion
> >
> > Some static tool reports
> > "DeviceLibLowestSupportedVersion" is used, but is
> uninitialized. It is
> > false positive reporting based because
> DeviceLibLowestSupportedVersion
> > will have assigned value after
> > FmpDeviceGetLowestSupportedVersion() returns SUCCESS.
> >
> > This patch refines the code to initialize
> > DeviceLibLowestSupportedVersion to
> DEFAULT_LOWESTSUPPORTEDVERSION
> > before calling FmpDeviceGetLowestSupportedVersion().
> > It can pass the static tool's check.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> > FmpDevicePkg/FmpDxe/FmpDxe.c | 6 ++----
> > 1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> > b/FmpDevicePkg/FmpDxe/FmpDxe.c index
> > b25a1511c70c..4868f80de2b9 100644
> > --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> > +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> > @@ -229,10 +229,8 @@ GetLowestSupportedVersion (
> > //
> > // Check the FmpDeviceLib
> > //
> > - Status = FmpDeviceGetLowestSupportedVersion
> > (&DeviceLibLowestSupportedVersion);
> > - if (EFI_ERROR (Status)) {
> > - DeviceLibLowestSupportedVersion =
> > DEFAULT_LOWESTSUPPORTEDVERSION;
> > - }
> > + DeviceLibLowestSupportedVersion =
> > DEFAULT_LOWESTSUPPORTEDVERSION;
> > + FmpDeviceGetLowestSupportedVersion
> > (&DeviceLibLowestSupportedVersion);
> >
> > if (DeviceLibLowestSupportedVersion > ReturnLsv) {
> > ReturnLsv = DeviceLibLowestSupportedVersion;
> > --
> > 2.7.0.windows.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-07-30 5:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-30 2:51 [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Initialize DeviceLibLowestSupportedVersion Zeng, Star
2018-07-30 5:35 ` Kinney, Michael D
2018-07-30 5:41 ` Zeng, Star
2018-07-30 5:43 ` Kinney, Michael D
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox