[PATCH v2 22/71] efi: Improve logging in efi_disk

[Heinrich Schuchardt]

On 1/13/23 23:40, Simon Glass wrote:
> Hi Tom, Heinrich,
>
> On Fri, 13 Jan 2023 at 13:40, Tom Rini <trini at konsulko.com> wrote:
>>
>> On Fri, Jan 13, 2023 at 09:35:36PM +0100, Heinrich Schuchardt wrote:
>>> On 1/8/23 03:49, Simon Glass wrote:
>>>> When this fails it can be time-consuming to debug. Add some debugging
>>>> to help with this. Also try to return error codes instead of just using
>>>> -1.
>>>>
>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>> ---
>>>>
>>>> (no changes since v1)
>>>>
>>>>    lib/efi_loader/efi_disk.c | 30 +++++++++++++++++++++---------
>>>>    1 file changed, 21 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>>>> index 7ea0334083f..37123dd2474 100644
>>>> --- a/lib/efi_loader/efi_disk.c
>>>> +++ b/lib/efi_loader/efi_disk.c
>>>> @@ -421,13 +421,16 @@ static efi_status_t efi_disk_add_dev(
>>>>
>>>>              if (!node) {
>>>>                      ret = EFI_OUT_OF_RESOURCES;
>>>> +                   log_debug("no node\n");
>>>
>>> Please, provide a descriptive message. I would not know what "no node"
>>> might mean if I were to read it.
>>>
>>> There is a reason why we set ret = EFI_OUT_OF_RESOURCES?
>>>
>>> The caller should know what this means.
>>
>> There's a fine balance to be struck here. An error message should be
>> grep'd through in the code, for debug messages. With CONFIG_LOG we're
>> already getting file, line and function.
>>
>
> Yes, and people need to read the code to figure out what went wrong. I
> did this patch because the EFI code returns the same error message for
> lots of situations and logging helped me work out what was wrong.
>
> But feel free to drop it if is isn't needed.

The patch is orthogonal to the rest of the series. I have set it to
"Changes requested", "Archived"

Best regards

Heinrich