From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=qin.long@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 1C0E6202E617A for ; Tue, 24 Oct 2017 01:12:01 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP; 24 Oct 2017 01:15:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,427,1503385200"; d="scan'208";a="141577410" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga004.jf.intel.com with ESMTP; 24 Oct 2017 01:15:44 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 24 Oct 2017 01:15:41 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by shsmsx102.ccr.corp.intel.com ([169.254.2.175]) with mapi id 14.03.0319.002; Tue, 24 Oct 2017 16:15:40 +0800 From: "Long, Qin" To: Laszlo Ersek , Peter Jones CC: "Ye, Ting" , "edk2-devel@lists.01.org" Thread-Topic: [edk2] [PATCH v1 1/1] CryptoPkg/BaseCryptLib: remove some duplicate initializations. Thread-Index: AQHTSbWcQM32uhiMfUebZF8zdgE456Lsc5OAgAATeYCABDwfsIABXPuAgACMe6A= Date: Tue, 24 Oct 2017 08:15:40 +0000 Message-ID: References: <20171020151018.785-1-pjones@redhat.com> <0a2c301c-58a8-ecf7-8d1a-40dd5ab8bc8e@redhat.com> <20171020182147.4rsisbduvvjweb2z@redhat.com> <12b81ebc-a0bb-4d67-9dbb-6687d036729b@redhat.com> In-Reply-To: <12b81ebc-a0bb-4d67-9dbb-6687d036729b@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v1 1/1] CryptoPkg/BaseCryptLib: remove some duplicate initializations. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Oct 2017 08:12:01 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable The patch was already push @b5a985ca9237b551618cd97b1b71af2fff55e209 I forgot to inform that. Thanks, Laszlo. Best Regards & Thanks, LONG, Qin -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Tuesday, October 24, 2017 3:51 PM To: Long, Qin ; Peter Jones Cc: Ye, Ting ; edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH v1 1/1] CryptoPkg/BaseCryptLib: remove some dupl= icate initializations. Qin, On 10/23/17 05:02, Long, Qin wrote: > This looks good to me. > Reviewed-by: Long Qin qin.long@intel.com Do you want me to push the patch, or do you prefer to push it yourself? Thanks! Laszlo > From: Peter Jones [mailto:pjones@redhat.com] > Sent: Saturday, October 21, 2017 2:22 AM > To: Laszlo Ersek > Cc: edk2-devel@lists.01.org; Shi, Steven ; Long,=20 > Qin ; Ye, Ting > Subject: Re: [edk2] [PATCH v1 1/1] CryptoPkg/BaseCryptLib: remove some du= plicate initializations. >=20 >> Assuming the maintainers are fine with the patch as well, I suggest=20 >> that they please replace the word "initializations" with=20 >> "assignments" in the subject, to be pedantic on the C-lang level. >=20 > Well, that's why I said "initializations" instead of "initializers",=20 > but if it's more clear to you, I'm fine with your way. >=20 >> (Side note: I would even move OldSize to a lot tighter scope: >> >>> diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c=20 >>> b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c >>> index d564591cb7f9..31a9ecd59ff6 100644 >>> --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c >>> +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c >>> @@ -477,7 +477,6 @@ Pkcs7GetCertificatesList ( >>> UINT8 *CertBuf; >>> UINT8 *OldBuf; >>> UINTN BufferSize; >>> - UINTN OldSize; >>> UINT8 *SingleCert; >>> UINTN CertSize; >>> >>> @@ -612,10 +611,11 @@ Pkcs7GetCertificatesList ( >>> >>> if (CtxChain !=3D NULL) { >>> BufferSize =3D sizeof (UINT8); >>> - OldSize =3D BufferSize; >>> CertBuf =3D NULL; >>> >>> for (Index =3D 0; ; Index++) { >>> + UINTN OldSize; >>> + >>> Status =3D X509PopCertificate (CtxChain, &SingleCert, &CertSize)= ; >>> if (!Status) { >>> break; >>> @@ -656,10 +656,11 @@ Pkcs7GetCertificatesList ( >>> >>> if (CtxUntrusted !=3D NULL) { >>> BufferSize =3D sizeof (UINT8); >>> - OldSize =3D BufferSize; >>> CertBuf =3D NULL; >>> >>> for (Index =3D 0; ; Index++) { >>> + UINTN OldSize; >>> + >>> Status =3D X509PopCertificate (CtxUntrusted, &SingleCert, &CertS= ize); >>> if (!Status) { >>> break; >> >> However, many edk2 maintainers don't like tight scoping like this.) >=20 > I had considered this and guessed it was probably against the coding=20 > style or it would have been done this way already. IMO it's better in ev= ery way. >=20 > -- > Peter >=20 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel