From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: philmd@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 23 Sep 2019 04:45:06 -0700 Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EDB3261D25 for ; Mon, 23 Sep 2019 11:45:05 +0000 (UTC) Received: by mail-wm1-f72.google.com with SMTP id 124so7258058wmz.1 for ; Mon, 23 Sep 2019 04:45:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=68RwDTN4vrX1rR5sIIjWXrEbGVLxuVTQW4SMXWTI0ko=; b=OYcSsb94DDf84zvO3wGDhPb9Ktw11fHUv5N2zYUAQISi+FcQUoPunAw/cxjWAjqSWX TEts9MA1EbUd3r048bDahsPY0g3ATnQel4Un33sKYOpS1K+BY+D700VdhFywDnjVUYj8 QRsxUdo0DhJV0MrwZwl2bbjN1Nd5ZnWTKedAcFhsbFgL2S44aDblXmNTn2SAX7+Pgiyd 2CSWXN70PTYD33ARRPYbnSsfk5Vp10Gqo5QzYyP8ZIL07cAn/1fCRn6H2E5B3gYBaKAv z5X4RsEFKlOdy8LlH1NmfOlrF4+VdAkaPMHGpfBQRYljTsMoRmDeFhlYOf/wf998SUUr IgFg== X-Gm-Message-State: APjAAAWD/jpN7ydGIZmYnvJD6kdnk/2Jk6BKHJTEIZlIQXsYCQuv0fE2 SOR9v+IPuwLTQ+BvZiFDtANkv9QlTtVdDhBWUqGK5wcRlLX/A+vRtd9fGMwcXRyUbg+hqLhEcde 8kvSMS4ac7qzaiw== X-Received: by 2002:a05:6000:188:: with SMTP id p8mr18649006wrx.220.1569239104741; Mon, 23 Sep 2019 04:45:04 -0700 (PDT) X-Google-Smtp-Source: APXvYqxNihBRH6Br7hq8s/nilOZd+on8dg/tmqkv2PqY+F5TYnrPTjGhC40JG3CsDIc9tKRKLsn55A== X-Received: by 2002:a05:6000:188:: with SMTP id p8mr18648983wrx.220.1569239104523; Mon, 23 Sep 2019 04:45:04 -0700 (PDT) Received: from [192.168.1.40] (240.red-88-21-68.staticip.rima-tde.net. [88.21.68.240]) by smtp.gmail.com with ESMTPSA id d28sm12076159wrb.95.2019.09.23.04.45.03 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 23 Sep 2019 04:45:03 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 10/35] MdeModulePkg/PlatformVarCleanupLib: fix HiiConstructConfigHdr() call To: devel@edk2.groups.io, lersek@redhat.com Cc: Hao A Wu , Jian J Wang , Liming Gao References: <20190917194935.24322-1-lersek@redhat.com> <20190917194935.24322-11-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: Date: Mon, 23 Sep 2019 13:45:02 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: <20190917194935.24322-11-lersek@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 9/17/19 9:49 PM, Laszlo Ersek wrote: > The HiiConstructConfigHdr() function takes the "DriverHandle" parameter in > order to fetch the device path from it, and then turn the device path into > PATH routing information. > > The HiiConstructConfigHdr() function is called from > VariableCleanupHiiExtractConfig(), which is only installed when "Type" is > "VarCleanupManually" in PlatformVarCleanup(). > > In that case, we create "Private->DriverHandle" as a new handle, and > install "mVarCleanupHiiVendorDevicePath" on it. Then we pass > "Private->DriverHandle" to HiiAddPackages(), which consumes the device > path for routing purposes. > > It follows that the "DriverHandle" argument pased to "passed" > HiiConstructConfigHdr() should be the same driver handle, for matching > routing. > > Currently we pass "Private->HiiHandle", which is clearly a typo, because > it is the return value of HiiAddPackages(), and stands for the published > HII package list. Phew... > Therefore this patch addresses an actual bug. > > The typo has not been flagged by compilers because the UEFI spec > regrettably defines both EFI_HANDLE and EFI_HII_HANDLE as (VOID*). > > Cc: Hao A Wu > Cc: Jian J Wang > Cc: Liming Gao > Signed-off-by: Laszlo Ersek > --- > > Notes: > build-tested only > > MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c > index 968c044a316a..3875d614bb41 100644 > --- a/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c > +++ b/MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c > @@ -609,7 +609,11 @@ VariableCleanupHiiExtractConfig ( > // Allocate and fill a buffer large enough to hold the template > // followed by "&OFFSET=0&WIDTH=WWWWWWWWWWWWWWWW" followed by a Null-terminator. > // > - ConfigRequestHdr = HiiConstructConfigHdr (&mVariableCleanupHiiGuid, mVarStoreName, Private->HiiHandle); > + ConfigRequestHdr = HiiConstructConfigHdr ( > + &mVariableCleanupHiiGuid, > + mVarStoreName, > + Private->DriverHandle > + ); > Size = (StrLen (ConfigRequestHdr) + 32 + 1) * sizeof (CHAR16); > ConfigRequest = AllocateZeroPool (Size); > ASSERT (ConfigRequest != NULL); > Reviewed-by: Philippe Mathieu-Daude