Qin, On 10/23/17 05:02, Long, Qin wrote: > This looks good to me. > Reviewed-by: Long Qin [email protected]<mailto:[email protected]>
Do you want me to push the patch, or do you prefer to push it yourself? Thanks! Laszlo > From: Peter Jones [mailto:[email protected]] > Sent: Saturday, October 21, 2017 2:22 AM > To: Laszlo Ersek <[email protected]> > Cc: [email protected]; Shi, Steven <[email protected]>; Long, Qin > <[email protected]>; Ye, Ting <[email protected]> > Subject: Re: [edk2] [PATCH v1 1/1] CryptoPkg/BaseCryptLib: remove some > duplicate initializations. > >> Assuming the maintainers are fine with the patch as well, I suggest that >> they please replace the word "initializations" with "assignments" in the >> subject, to be pedantic on the C-lang level. > > Well, that's why I said "initializations" instead of "initializers", but if > it's more clear to you, I'm fine with your way. > >> (Side note: I would even move OldSize to a lot tighter scope: >> >>> diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c >>> 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 != NULL) { >>> BufferSize = sizeof (UINT8); >>> - OldSize = BufferSize; >>> CertBuf = NULL; >>> >>> for (Index = 0; ; Index++) { >>> + UINTN OldSize; >>> + >>> Status = X509PopCertificate (CtxChain, &SingleCert, &CertSize); >>> if (!Status) { >>> break; >>> @@ -656,10 +656,11 @@ Pkcs7GetCertificatesList ( >>> >>> if (CtxUntrusted != NULL) { >>> BufferSize = sizeof (UINT8); >>> - OldSize = BufferSize; >>> CertBuf = NULL; >>> >>> for (Index = 0; ; Index++) { >>> + UINTN OldSize; >>> + >>> Status = X509PopCertificate (CtxUntrusted, &SingleCert, &CertSize); >>> if (!Status) { >>> break; >> >> However, many edk2 maintainers don't like tight scoping like this.) > > I had considered this and guessed it was probably against the coding style or > it would have been done this way already. IMO it's better in every way. > > -- > Peter > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
