From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [edk2-devel] [PATCH] NetworkPkg: Correct the length of EAP Identity when in ASCII format To: Clark-williams, Zachary ,devel@edk2.groups.io From: "Li, Yi" X-Originating-Location: CN (192.102.204.51) X-Originating-Platform: Windows Chrome 114 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Mon, 19 Jun 2023 21:49:02 -0700 References: In-Reply-To: Message-ID: <8643.1687236542190595908@groups.io> Content-Type: multipart/alternative; boundary="1oMrFegkYdeTBrhFPtdR" --1oMrFegkYdeTBrhFPtdR Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Zachary, Thanks for review. >=20 > The protocol has changes since ADL from PlatSapmle to an advanced feature > and the Protocol has shifted into EDK2, so the protocol name needs to be > updated: > + Status =3D gBS->LocateProtocol (& gWiFiProfileSyncProtocolGuid , NULL, > (VOID **) &WiFiProfileSyncProtocol); > Status =3D gBS->LocateProtocol (& gEdkiiWiFiProfileSyncProtocolGuid , NUL= L, > (VOID **)&WiFiProfileSyncProtocol); >=20 > The locate protocol status check is enough and we do not need to add the > NULL check too, we can remove that to keep it lighter. > +=C2=A0=C2=A0=C2=A0 if (!EFI_ERROR (Status) && WiFiProfileSyncProtocol != =3D NULL ) { >=20 >=20 Agree with those changes, please check latest V2 patch or this PR: https://= github.com/tianocore/edk2/pull/4561 >=20 > Can we clean up the second locate protocol and bring the Identity allocat= e > above the protocol check, and bring the two conditions for EapIdentity > copied to Identity into the added protocol check condition. > Here is a view of what I am thinking for consolidation. >=20 We need to get the Identity size before AllocateZeroPool() , not feasible h= ere IMO. --1oMrFegkYdeTBrhFPtdR Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi Zachary,

Thanks for review.
The protocol has= changes since ADL from PlatSapmle to an advanced feature and the Protocol = has shifted into EDK2, so the protocol name needs to be updated:
+  = ;Status =3D gBS->LocateProtocol (&gWiFiProfileSyncProtocolGuid, NUL= L, (VOID **) &WiFiProfileSyncProtocol);
  =   Status =3D gBS->LocateProtocol (&gEdkiiWiFiProfileSyncProtocolGuid, NULL, (VOID **)&WiFiProfileSyncProtocol);
 
The locate proto= col status check is enough and we do not need to add the NULL check too, we= can remove that to keep it lighter.
+  = ;  if (!EFI_ERROR (Status) && WiFiProfileSyncProtocol !=3D NULL) = {
Agree with those changes, please check latest V2 patch or this PR: https://github.com/tianocore/edk2/pull/4561

Can we clean up = the second locate protocol and bring the Identity allocate above the protoc= ol check, and bring the two conditions for EapIdentity copied to Identity i= nto the added protocol check condition.
Here is a view o= f what I am thinking for consolidation.
We need to get = the Identity size before AllocateZeroPool(), not feasible here IMO.<= /span>
--1oMrFegkYdeTBrhFPtdR--