Good comments and sure to add (VOID **) cast. Thanks for your Tested-by.
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Tuesday, July 21, 2015 8:39 PM
To: Zeng, Star; edk2-de...@ml01.01.org
Cc: Yao, Jiewen; Alex Williamson
Subject: Re: [edk2] [PATCH 2/2] SecurityPkg AuthVariableLib: Correct address
pointers data
Indeed, this bug breaks the installation of Windows 8.1 guests on top of OVMF.
(The installer crashes while copying files.)
Alex (Cc'd) reported this to me in private. I bisected the issue to the
variable driver unification patches, and then found this new series on the list.
One comment below:
On 07/21/15 11:01, Star Zeng wrote:
> Originally, the double pointer (VOID **) is not correct for convert
> address pointers, and also some address pointers were missing.
>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.z...@intel.com>
> ---
> SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 19
> +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> index 02df309..d102219 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> @@ -101,7 +101,7 @@ VARIABLE_ENTRY_PROPERTY mAuthVarEntry[] = {
> },
> };
>
> -VOID *mAddressPointer[3];
> +VOID **mAuthVarAddressPointer[10];
>
> AUTH_VAR_LIB_CONTEXT_IN *mAuthVarLibContextIn = NULL;
>
> @@ -406,11 +406,18 @@ AuthVariableLibInitialize (
> AuthVarLibContextOut->StructSize = sizeof (AUTH_VAR_LIB_CONTEXT_OUT);
> AuthVarLibContextOut->AuthVarEntry = mAuthVarEntry;
> AuthVarLibContextOut->AuthVarEntryCount = sizeof (mAuthVarEntry) /
> sizeof (mAuthVarEntry[0]);
> - mAddressPointer[0] = mHashCtx;
> - mAddressPointer[1] = mPubKeyStore;
> - mAddressPointer[2] = mCertDbStore;
> - AuthVarLibContextOut->AddressPointer = mAddressPointer;
> - AuthVarLibContextOut->AddressPointerCount = sizeof
> (mAddressPointer) / sizeof (mAddressPointer[0]);
> + mAuthVarAddressPointer[0] = &mPubKeyStore;
> + mAuthVarAddressPointer[1] = &mCertDbStore;
> + mAuthVarAddressPointer[2] = &mHashCtx; mAuthVarAddressPointer[3] =
> + &mAuthVarLibContextIn;
Please cast all of these to (VOID **), same as below; otherwise gcc complains
with:
assignment from incompatible pointer type
If you fix that up, you can add my:
Tested-by: Laszlo Ersek <ler...@redhat.com>
to both patches in this series.
Thanks
Laszlo
> + mAuthVarAddressPointer[4] = (VOID **)
> + &(mAuthVarLibContextIn->FindVariable),
> + mAuthVarAddressPointer[5] = (VOID **)
> + &(mAuthVarLibContextIn->FindNextVariable),
> + mAuthVarAddressPointer[6] = (VOID **)
> + &(mAuthVarLibContextIn->UpdateVariable),
> + mAuthVarAddressPointer[7] = (VOID **)
> + &(mAuthVarLibContextIn->GetScratchBuffer),
> + mAuthVarAddressPointer[8] = (VOID **)
> + &(mAuthVarLibContextIn->CheckRemainingSpaceForConsistency),
> + mAuthVarAddressPointer[9] = (VOID **)
> + &(mAuthVarLibContextIn->AtRuntime),
> + AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
> + AuthVarLibContextOut->AddressPointerCount = sizeof
> + (mAuthVarAddressPointer) / sizeof (mAuthVarAddressPointer[0]);
>
> return Status;
> }
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
