From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web10.28920.1574364126645640803 for ; Thu, 21 Nov 2019 11:22:06 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=y4a0XkB/; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: michael.a.kubacki@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Nov 2019 11:22:06 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,226,1571727600"; d="scan'208";a="259465917" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by FMSMGA003.fm.intel.com with ESMTP; 21 Nov 2019 11:22:06 -0800 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 21 Nov 2019 11:22:06 -0800 Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 21 Nov 2019 11:22:05 -0800 Received: from NAM04-SN1-obe.outbound.protection.outlook.com (104.47.44.59) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 21 Nov 2019 11:22:05 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LORUPSE1bjG5jY4jEFIchyDhh3dbcQAPWDAhOXDNBfzX1VKUoTSz01eQ8FvBYI+hRThckz6kUV4faGESxAXVDYCQSbgLVEQseyBEyb5SC5ZV2Gn7b36cTWt/WmmH36o97380L1ZRWWcIPwJcsWqO3Ue5mC2+vubise4WNWc+4kaiQn5ADI6Uo9MnaYr3Q2i1V9/dgDuclFotkI0RjxTO5w/IH8Y8q/WfNGdIzHVI/Y84C0vnexzRnVKsjvIhzkZBK9HKHhnmD7Ubbwb4nYpKG5OajLvCt3VxhnjzMWpm00thZkTxHY4mwnIVlGwLbouohnwuUdUYKMx5SjcauQcmQA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jAOn+rSDykoE0MhitX57XVJfrFNIqlddLRdhgHoO0uE=; b=WvG65F32Yh7FvyWk2zsFVMl3DHaLqmdhDPBLexZ5IkJGhL7za/imTQ8KpH3DTmE001wVlVf2nOHCPO1bWGeC7qghSr8nvoanlXipkK+caFQRUKKypNjlsOwSgr+giMpbf3TczL0uQNer7omQyXO5CtQq332Tb1WX4Drki16Ltg+RvFkErSQgE237rQM7OVIvTA+6y5RBjW6vpstToUxDww/zohsQdQ0+m0uHcb157xbfyjIj+c8ln6CVdQ1jn5hGFHyE0KIyAtgyTRdz21Kbs/Cz65+IKFORxgUJ6RuWQIUxMzYrNeeLl2O626pSlS2+Igs1EbVc+ThmGl2MyL/zDg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jAOn+rSDykoE0MhitX57XVJfrFNIqlddLRdhgHoO0uE=; b=y4a0XkB//JlGcXrMSs65ro3+AhPVmVhh4c8b6Vzth100grvm4XfcvSewk4QG003CYkQ6hhYWpwv013TV75DOQkflpSNiVt9sP3TK+HXzfRilAvIizzXosfWs1uLWF0gxdJPQ+jNywiFLzCO6vnLu5fnov6W6MWMjH2SH7tv7JwU= Received: from BY5PR11MB4484.namprd11.prod.outlook.com (52.132.254.155) by BY5PR11MB4370.namprd11.prod.outlook.com (52.132.254.148) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2474.17; Thu, 21 Nov 2019 19:22:03 +0000 Received: from BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::a114:604b:7ca3:5420]) by BY5PR11MB4484.namprd11.prod.outlook.com ([fe80::a114:604b:7ca3:5420%7]) with mapi id 15.20.2474.019; Thu, 21 Nov 2019 19:22:03 +0000 From: "Kubacki, Michael A" To: =?iso-8859-1?Q?Philippe_Mathieu-Daud=E9?= , "devel@edk2.groups.io" CC: "Gao, Liming" , "Kinney, Michael D" , "Wang, Jian J" , "Wu, Hao A" Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Initialize local variable Thread-Topic: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Initialize local variable Thread-Index: AQHVoIcBVsH7WGVFB0mI+IcRU+efuaeV3hoQ Date: Thu, 21 Nov 2019 19:22:03 +0000 Message-ID: References: <20191121023256.24820-1-michael.a.kubacki@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzA3YzQ1NzYtNmEyMy00YWJkLWExMTQtMDFmYzAzOTlkYWYwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoibmtSa1JvRkNqU3lPSFVxaUpKZ0htQlwvNElkZWdQK3lcL2ltdXZJdmRnN0tqZHB4QndGeVpcLzFvU3NlVjVaN1lSbyJ9 dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: spf=none (sender IP is ) smtp.mailfrom=michael.a.kubacki@intel.com; x-originating-ip: [134.134.136.217] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 2b885316-c3d3-44cc-0f98-08d76eb8172c x-ms-traffictypediagnostic: BY5PR11MB4370: x-ms-exchange-purlcount: 1 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:2733; x-forefront-prvs: 0228DDDDD7 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(366004)(346002)(39850400004)(396003)(376002)(136003)(189003)(199004)(13464003)(508600001)(316002)(26005)(8936002)(2501003)(8676002)(11346002)(446003)(76176011)(110136005)(25786009)(14454004)(7736002)(305945005)(6306002)(74316002)(53546011)(54906003)(102836004)(6506007)(33656002)(81166006)(14444005)(55016002)(256004)(9686003)(66066001)(6436002)(229853002)(6246003)(86362001)(2906002)(107886003)(186003)(71200400001)(71190400001)(7696005)(52536014)(66556008)(66476007)(5660300002)(66446008)(66946007)(64756008)(99286004)(4326008)(76116006)(81156014)(3846002)(6116002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY5PR11MB4370;H:BY5PR11MB4484.namprd11.prod.outlook.com;FPR:;SPF:None;LANG:en;PTR:InfoNoRecords;MX:1;A:1; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: pMsjZriwOATIvuyiohXsq+cG9EhDL2S25OjoYgEgvPivaeDKIUpHjf96onLYDjPYPg+KHKYuNe9Q13GZ4lvEH2qvWdStxdT714Be63L/q8JKJmEq5KU0P/wwiqmIfw0Rt8eTHYTjwDwfzWG//CfKXyd9uJrV0ADcWOoVV21wCCNhQlHBxCvMyQg62PA9v1h8hlDZkn0oCohrf8w8A+SzMs9U9TpNPUF42oWMaomYkyfyjCXj/Z4aL4YxHmNfAP7Xi2eweL86CqJgrcLqQKCqtRMJGthPImFkLSTdBY/RCHJCMcdemC6b7VJ/N2/hDQnrElHmBCuCj5NcGjfo1xAurLoq2QxWXsuZ54S/IfZ3lem4/xOU2wDhfgCFB4agWJQWQhLHcKTQh5g9Q7nPgqVbTfYUjrjXBS2YCcce9VJUZRrDIxSOxQWk7xQ5DnkZozKKL1iIlepRS7IvPvJE0tVxpYH7A5DIgvP3Dr1kPFLVpBs= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 2b885316-c3d3-44cc-0f98-08d76eb8172c X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Nov 2019 19:22:03.6236 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: nL+nGM4r+pqNIKI0xHKeABFnbnVK2Xj5HikomSl4nPUk1MJP1VA8ImDX1vtVtiEG57HkyA4X1uNV2hHQvjev3Baekyu0rXOetYB1wIs6mhw= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4370 Return-Path: michael.a.kubacki@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi Philippe, I will split the change into two patches. In the current implementation of VariableSmmRuntimeDxe.c there is not an actual problem. The Status variable is set to EFI_NOT_FOUND and RtPtrTrack is only used if Status is not set to an error code which requires the RtPtrTrack variable to be set. Given that this is a local variable, the initial value is undefined so I'd prefer to solve this = by initializing the variable value. This will better accommodate future changes to the function by having a reasonable default value. Regarding your suggestion, the expected behavior is that RtPtrTrack.CurrPtr is assigned in the for loop and the value is used in the if block after the for block so jumping to the Done label would not be appropriate. The variable would also still be uninitialized. Thanks, Michael > -----Original Message----- > From: Philippe Mathieu-Daud=E9 > Sent: Thursday, November 21, 2019 8:16 AM > To: devel@edk2.groups.io; Kubacki, Michael A > > Cc: Gao, Liming ; Kinney, Michael D > ; Wang, Jian J ; Wu, > Hao A > Subject: Re: [edk2-devel] [PATCH V1 1/1] MdeModulePkg/Variable: Initializ= e > local variable >=20 > Hi Michael, >=20 > On 11/21/19 3:32 AM, Kubacki, Michael A wrote: > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2364 > > > > Fixes a new build warning in VS2012 introduced in f8ff4cca7c. > > > > This patch initializes the local variable "Variable" in > > VariableServiceGetNextVariableInternal () and the local variable > > "RtPtrTrack" in FindVariableInRuntimeCache (). > > This enusres the pointers in the structures are initialized >=20 > Typo "this ensures" >=20 > > in the case no variable stores exist in the list of variable stores. > > > > Cc: Liming Gao > > Cc: Michael D Kinney > > Cc: Jian J Wang > > Cc: Hao A Wu > > Signed-off-by: Michael Kubacki > > --- > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c | = 2 > ++ > > > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe. > c | 2 ++ > > 2 files changed, 4 insertions(+) > > > > diff --git > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > > index d458f1c608..f6d187543d 100644 > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableParsing.c > > @@ -551,6 +551,8 @@ VariableServiceGetNextVariableInternal ( > > return EFI_INVALID_PARAMETER; > > } > > > > + ZeroMem (&Variable, sizeof (Variable)); > > + >=20 > I agree with this change. >=20 > > // Check if the variable exists in the given variable store list > > for (StoreType =3D (VARIABLE_STORE_TYPE) 0; StoreType < > VariableStoreTypeMax; StoreType++) { > > if (VariableStoreList[StoreType] =3D=3D NULL) { diff --git > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > e.c > > > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > e.c > > index d525998ae3..2cf0ed32ae 100644 > > --- > > > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > e.c > > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > e > > +++ .c > > @@ -590,6 +590,8 @@ FindVariableInRuntimeCache ( > > return EFI_INVALID_PARAMETER; > > } > > > > + ZeroMem (&RtPtrTrack, sizeof (RtPtrTrack)); > > + > > // > > // The UEFI specification restricts Runtime Services callers from i= nvoking > the same or certain other Runtime Service > > // functions prior to completion and return from a previous > > Runtime Service call. These restrictions prevent > > >=20 > Here this seems overkill, what about: >=20 > -- >8 -- > --- > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > e.c > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx > e.c > @@ -626,6 +626,10 @@ FindVariableInRuntimeCache ( > } > } >=20 > + if (RtPtrTrack.CurrPtr) { > + goto Done; > + } > + > if (!EFI_ERROR (Status)) { > // > // Get data size >=20 > --- >=20 > Can you split this patch in 2? > If so you can add to the 1st part: > Reviewed-by: Philippe Mathieu-Daude