* [PATCH 1/2] ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr parsing
2016-09-08 16:14 [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Laszlo Ersek
@ 2016-09-08 16:14 ` Laszlo Ersek
2016-09-08 16:14 ` [PATCH 2/2] ShellPkg/UefiHandleParsingLib: fix retval for empty child controller array Laszlo Ersek
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2016-09-08 16:14 UTC (permalink / raw)
To: edk2-devel; +Cc: Jaben Carsey, Ruiyu Ni, Tapan Shah
"MatchingHandleCount" is an output parameter of
ParseHandleDatabaseForChildControllers().
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Tapan Shah <tapandshah@hpe.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ShellPkg/Include/Library/HandleParsingLib.h | 4 ++--
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ShellPkg/Include/Library/HandleParsingLib.h b/ShellPkg/Include/Library/HandleParsingLib.h
index 441f65fffe83..79dcc9cc5a55 100644
--- a/ShellPkg/Include/Library/HandleParsingLib.h
+++ b/ShellPkg/Include/Library/HandleParsingLib.h
@@ -303,24 +303,24 @@ ParseHandleDatabaseForChildDevices(
);
/**
Gets handles for any child controllers of the passed in controller.
@param[in] ControllerHandle The handle of the "parent controller".
- @param[in] MatchingHandleCount The pointer to the number of handles in
+ @param[out] MatchingHandleCount The pointer to the number of handles in
MatchingHandleBuffer on return.
@param[out] MatchingHandleBuffer The buffer containing handles on a successful
return.
@retval EFI_SUCCESS The operation was successful.
@sa ParseHandleDatabaseByRelationship
**/
EFI_STATUS
EFIAPI
ParseHandleDatabaseForChildControllers(
IN CONST EFI_HANDLE ControllerHandle,
- IN UINTN *MatchingHandleCount,
+ OUT UINTN *MatchingHandleCount,
OUT EFI_HANDLE **MatchingHandleBuffer OPTIONAL
);
/**
Function to retrieve the human-friendly index of a given handle. If the handle
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index 3fb55df8cc14..e11a3ccceab3 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -2720,25 +2720,25 @@ ParseHandleDatabaseByRelationship (
}
/**
Gets handles for any child controllers of the passed in controller.
@param[in] ControllerHandle The handle of the "parent controller"
- @param[in] MatchingHandleCount Pointer to the number of handles in
+ @param[out] MatchingHandleCount Pointer to the number of handles in
MatchingHandleBuffer on return.
@param[out] MatchingHandleBuffer Buffer containing handles on a successful
return.
@retval EFI_SUCCESS The operation was sucessful.
**/
EFI_STATUS
EFIAPI
ParseHandleDatabaseForChildControllers(
IN CONST EFI_HANDLE ControllerHandle,
- IN UINTN *MatchingHandleCount,
+ OUT UINTN *MatchingHandleCount,
OUT EFI_HANDLE **MatchingHandleBuffer OPTIONAL
)
{
EFI_STATUS Status;
UINTN HandleIndex;
UINTN DriverBindingHandleCount;
--
2.9.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] ShellPkg/UefiHandleParsingLib: fix retval for empty child controller array
2016-09-08 16:14 [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Laszlo Ersek
2016-09-08 16:14 ` [PATCH 1/2] ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr parsing Laszlo Ersek
@ 2016-09-08 16:14 ` Laszlo Ersek
2016-09-08 16:37 ` [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Carsey, Jaben
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2016-09-08 16:14 UTC (permalink / raw)
To: edk2-devel; +Cc: Jaben Carsey, Ruiyu Ni, Tapan Shah
The ParseHandleDatabaseForChildControllers() function intends to work like
this:
(1) It allocates a "HandleBufferForReturn" local array that's guaranteed
to be big enough for all found handles,
(2) it collects the handles, both counting them in the (mandatory)
"MatchingHandleCount" output parameter, and saving them in the local
"HandleBufferForReturn" array,
(3) if the caller is not interested in the actual handles, then
"HandleBufferForReturn" is released,
(4) if the caller is interested in the handles, and we've found some, then
"HandleBufferForReturn" is passed out through the
"MatchingHandleBuffer" output parameter,
(5) if the caller is interested in the actual handles, but we've found
none, then the "MatchingHandleBuffer" output parameter is set to NULL.
The ASSERT() at the end of the function makes this clear, but the
implementation does not conform to (5). Fix it.
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Tapan Shah <tapandshah@hpe.com>
Reported-by: Tapan Shah <tapandshah@hpe.com>
Ref: https://tianocore.acgmultimedia.com/show_bug.cgi?id=112
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index e11a3ccceab3..695d090926e1 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -2799,17 +2799,24 @@ ParseHandleDatabaseForChildControllers(
FreePool (ChildControllerHandleBuffer);
}
FreePool (DriverBindingHandleBuffer);
+ if (MatchingHandleBuffer == NULL || *MatchingHandleCount == 0) {
+ //
+ // The caller is not interested in the actual handles, or we've found none.
+ //
+ FreePool (HandleBufferForReturn);
+ HandleBufferForReturn = NULL;
+ }
+
if (MatchingHandleBuffer != NULL) {
*MatchingHandleBuffer = HandleBufferForReturn;
- } else {
- FreePool(HandleBufferForReturn);
}
+
ASSERT ((MatchingHandleBuffer == NULL) ||
(*MatchingHandleCount == 0 && *MatchingHandleBuffer == NULL) ||
(*MatchingHandleCount != 0 && *MatchingHandleBuffer != NULL));
return (EFI_SUCCESS);
}
--
2.9.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
2016-09-08 16:14 [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Laszlo Ersek
2016-09-08 16:14 ` [PATCH 1/2] ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr parsing Laszlo Ersek
2016-09-08 16:14 ` [PATCH 2/2] ShellPkg/UefiHandleParsingLib: fix retval for empty child controller array Laszlo Ersek
@ 2016-09-08 16:37 ` Carsey, Jaben
2016-09-08 17:08 ` Shah, Tapan
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Carsey, Jaben @ 2016-09-08 16:37 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org; +Cc: Ni, Ruiyu, Carsey, Jaben
Thanks and for the Series.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, September 08, 2016 9:14 AM
> To: edk2-devel@ml01.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [edk2] [PATCH 0/2] ShellPkg: fix assertion failure encountered with
> "devtree" and "dh -d -v"
> Importance: High
>
> Tapan reported
> <https://tianocore.acgmultimedia.com/show_bug.cgi?id=112>. Since I had
> encountered the problem myself, and now managed to find a tiny time
> slot, I looked into it. Patch #2 is the fix; patch #1 cleans up a small
> wart that I came across while studying the code.
>
> Public branch: <https://github.com/lersek/edk2/commits/childctrl>.
>
> Tapan, can you please test the series, and respond with your Tested-by?
> Thanks!
>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
>
> Cheers,
> Laszlo
>
> Laszlo Ersek (2):
> ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr
> parsing
> ShellPkg/UefiHandleParsingLib: fix retval for empty child controller
> array
>
> ShellPkg/Include/Library/HandleParsingLib.h | 4 ++--
> ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 15
> +++++++++++----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> --
> 2.9.2
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
2016-09-08 16:14 [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Laszlo Ersek
` (2 preceding siblings ...)
2016-09-08 16:37 ` [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Carsey, Jaben
@ 2016-09-08 17:08 ` Shah, Tapan
2016-09-08 17:09 ` Shah, Tapan
2016-09-08 18:46 ` Laszlo Ersek
5 siblings, 0 replies; 8+ messages in thread
From: Shah, Tapan @ 2016-09-08 17:08 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org; +Cc: Jaben Carsey, Ruiyu Ni
Missing company's copyright message in both changed filed. Code works fine on NT32.
Reviewed-by: Tapan Shah <tapandshah@hpe.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Thursday, September 08, 2016 11:14 AM
To: edk2-devel@ml01.01.org
Cc: Jaben Carsey <jaben.carsey@intel.com>; Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [edk2] [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
Tapan reported
<https://tianocore.acgmultimedia.com/show_bug.cgi?id=112>. Since I had
encountered the problem myself, and now managed to find a tiny time
slot, I looked into it. Patch #2 is the fix; patch #1 cleans up a small
wart that I came across while studying the code.
Public branch: <https://github.com/lersek/edk2/commits/childctrl>.
Tapan, can you please test the series, and respond with your Tested-by?
Thanks!
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Tapan Shah <tapandshah@hpe.com>
Cheers,
Laszlo
Laszlo Ersek (2):
ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr
parsing
ShellPkg/UefiHandleParsingLib: fix retval for empty child controller
array
ShellPkg/Include/Library/HandleParsingLib.h | 4 ++--
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 15 +++++++++++----
2 files changed, 13 insertions(+), 6 deletions(-)
--
2.9.2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
2016-09-08 16:14 [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Laszlo Ersek
` (3 preceding siblings ...)
2016-09-08 17:08 ` Shah, Tapan
@ 2016-09-08 17:09 ` Shah, Tapan
2016-09-08 18:35 ` Laszlo Ersek
2016-09-08 18:46 ` Laszlo Ersek
5 siblings, 1 reply; 8+ messages in thread
From: Shah, Tapan @ 2016-09-08 17:09 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@ml01.01.org; +Cc: Jaben Carsey, Ruiyu Ni
Typo..... Missing company's copyright message in both changed files*
-----Original Message-----
From: Shah, Tapan
Sent: Thursday, September 08, 2016 12:09 PM
To: 'Laszlo Ersek' <lersek@redhat.com>; edk2-devel@ml01.01.org
Cc: Jaben Carsey <jaben.carsey@intel.com>; Ruiyu Ni <ruiyu.ni@intel.com>
Subject: RE: [edk2] [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
Missing company's copyright message in both changed filed. Code works fine on NT32.
Reviewed-by: Tapan Shah <tapandshah@hpe.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
Sent: Thursday, September 08, 2016 11:14 AM
To: edk2-devel@ml01.01.org
Cc: Jaben Carsey <jaben.carsey@intel.com>; Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [edk2] [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
Tapan reported
<https://tianocore.acgmultimedia.com/show_bug.cgi?id=112>. Since I had encountered the problem myself, and now managed to find a tiny time slot, I looked into it. Patch #2 is the fix; patch #1 cleans up a small wart that I came across while studying the code.
Public branch: <https://github.com/lersek/edk2/commits/childctrl>.
Tapan, can you please test the series, and respond with your Tested-by?
Thanks!
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Tapan Shah <tapandshah@hpe.com>
Cheers,
Laszlo
Laszlo Ersek (2):
ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr
parsing
ShellPkg/UefiHandleParsingLib: fix retval for empty child controller
array
ShellPkg/Include/Library/HandleParsingLib.h | 4 ++--
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 15 +++++++++++----
2 files changed, 13 insertions(+), 6 deletions(-)
--
2.9.2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
2016-09-08 17:09 ` Shah, Tapan
@ 2016-09-08 18:35 ` Laszlo Ersek
0 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2016-09-08 18:35 UTC (permalink / raw)
To: Shah, Tapan, edk2-devel@ml01.01.org; +Cc: Jaben Carsey, Ruiyu Ni
On 09/08/16 19:09, Shah, Tapan wrote:
> Typo..... Missing company's copyright message in both changed files*
Bah, I don't care about that for files that exist, the git log is just fine for that. :) I am aware that a copyright notice is required for new files, and I do comply with that absolutely. I also never touch copyright lines belonging to other companies (big no-no).
But, I generally don't litter my patches with RH copyright lines (or copyright line updates) for files that already exist -- other companies may require their associates to do so (I recall such hunks from Intel), but we don't do that (not just in edk2, but as far as I am aware, in any of the other projects where we modify files -- QEMU and the Linux kernel are the most direct examples I can think of).
For example, in Linux, the "arch/x86/kvm/vmx.c" file still says:
/*
* Kernel-based Virtual Machine driver for Linux
*
* This module enables machines with Intel VT-x extensions to run virtual
* machines without emulation or binary translation.
*
* Copyright (C) 2006 Qumranet, Inc.
* Copyright 2010 Red Hat, Inc. and/or its affiliates.
*
* Authors:
* Avi Kivity <avi@qumranet.com>
* Yaniv Kamay <yaniv@qumranet.com>
*
* This work is licensed under the terms of the GNU GPL, version 2. See
* the COPYING file in the top-level directory.
*
*/
Funnily enough, the Red Hat line was added in kernel commit 221d059d15f1c ("KVM: Update Red Hat copyrights") in 2010, even though RH acquired Qumranet in 2008 (according to Wikipedia).
But, I digress. I think the current patches satisfy "ShellPkg/Contributions.txt". I do thank you for pointing this out: if I were responsible for sticking the explicit (C) RH notice in every preexistent file I touch, I'd definitely fix up the patches now.
Thank you!
Laszlo
> -----Original Message-----
> From: Shah, Tapan
> Sent: Thursday, September 08, 2016 12:09 PM
> To: 'Laszlo Ersek' <lersek@redhat.com>; edk2-devel@ml01.01.org
> Cc: Jaben Carsey <jaben.carsey@intel.com>; Ruiyu Ni <ruiyu.ni@intel.com>
> Subject: RE: [edk2] [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
>
> Missing company's copyright message in both changed filed. Code works fine on NT32.
>
> Reviewed-by: Tapan Shah <tapandshah@hpe.com>
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Thursday, September 08, 2016 11:14 AM
> To: edk2-devel@ml01.01.org
> Cc: Jaben Carsey <jaben.carsey@intel.com>; Ruiyu Ni <ruiyu.ni@intel.com>
> Subject: [edk2] [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
>
> Tapan reported
> <https://tianocore.acgmultimedia.com/show_bug.cgi?id=112>. Since I had encountered the problem myself, and now managed to find a tiny time slot, I looked into it. Patch #2 is the fix; patch #1 cleans up a small wart that I came across while studying the code.
>
> Public branch: <https://github.com/lersek/edk2/commits/childctrl>.
>
> Tapan, can you please test the series, and respond with your Tested-by?
> Thanks!
>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
>
> Cheers,
> Laszlo
>
> Laszlo Ersek (2):
> ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr
> parsing
> ShellPkg/UefiHandleParsingLib: fix retval for empty child controller
> array
>
> ShellPkg/Include/Library/HandleParsingLib.h | 4 ++--
> ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 15 +++++++++++----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
> --
> 2.9.2
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v"
2016-09-08 16:14 [PATCH 0/2] ShellPkg: fix assertion failure encountered with "devtree" and "dh -d -v" Laszlo Ersek
` (4 preceding siblings ...)
2016-09-08 17:09 ` Shah, Tapan
@ 2016-09-08 18:46 ` Laszlo Ersek
5 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2016-09-08 18:46 UTC (permalink / raw)
To: edk2-devel; +Cc: Jaben Carsey, Ruiyu Ni
On 09/08/16 18:14, Laszlo Ersek wrote:
> Tapan reported
> <https://tianocore.acgmultimedia.com/show_bug.cgi?id=112>. Since I had
> encountered the problem myself, and now managed to find a tiny time
> slot, I looked into it. Patch #2 is the fix; patch #1 cleans up a small
> wart that I came across while studying the code.
>
> Public branch: <https://github.com/lersek/edk2/commits/childctrl>.
>
> Tapan, can you please test the series, and respond with your Tested-by?
> Thanks!
>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Tapan Shah <tapandshah@hpe.com>
>
> Cheers,
> Laszlo
>
> Laszlo Ersek (2):
> ShellPkg/UefiHandleParsingLib: fix IN/OUT notation in child ctrlr
> parsing
> ShellPkg/UefiHandleParsingLib: fix retval for empty child controller
> array
>
> ShellPkg/Include/Library/HandleParsingLib.h | 4 ++--
> ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 15 +++++++++++----
> 2 files changed, 13 insertions(+), 6 deletions(-)
>
Thanks everyone for the reviews & testing, series pushed as
1b3be4a12e7d..7eb3bb6c552d.
Cheers!
Laszlo
^ permalink raw reply [flat|nested] 8+ messages in thread