Use more explicit goto statements to handle common error code
path instead of a lot of if/else.
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
xen/common/efi/boot.c | 80 +++++++++++++++++++++++--------------------
1 file changed, 43 insertions(+), 37 deletions(-)
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 4cbf1aa894..f6e8d4726d 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -792,6 +792,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
if ( !name )
PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
+
+ what = L"Open";
if ( dir_handle )
ret = dir_handle->Open(dir_handle, &FileHandle, name,
EFI_FILE_MODE_READ, 0);
@@ -800,54 +802,58 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
if ( file == &cfg && ret == EFI_NOT_FOUND )
return false;
if ( EFI_ERROR(ret) )
- what = L"Open";
- else
- ret = FileHandle->SetPosition(FileHandle, -1);
+ goto fail;
+
+ what = L"Seek";
+ ret = FileHandle->SetPosition(FileHandle, -1);
if ( EFI_ERROR(ret) )
- what = what ?: L"Seek";
- else
- ret = FileHandle->GetPosition(FileHandle, &size);
+ goto fail;
+
+ what = L"Get size";
+ ret = FileHandle->GetPosition(FileHandle, &size);
if ( EFI_ERROR(ret) )
- what = what ?: L"Get size";
- else
- ret = FileHandle->SetPosition(FileHandle, 0);
+ goto fail;
+
+ what = L"Seek";
+ ret = FileHandle->SetPosition(FileHandle, 0);
if ( EFI_ERROR(ret) )
- what = what ?: L"Seek";
- else
- {
- file->addr = min(1UL << (32 + PAGE_SHIFT),
- HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
- ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
- PFN_UP(size), &file->addr);
- }
+ goto fail;
+
+ what = L"Allocation";
+ file->addr = min(1UL << (32 + PAGE_SHIFT),
+ HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
+ ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
+ PFN_UP(size), &file->addr);
if ( EFI_ERROR(ret) )
- what = what ?: L"Allocation";
- else
- {
- file->need_to_free = true;
- file->size = size;
- handle_file_info(name, file, options);
+ goto fail;
- ret = FileHandle->Read(FileHandle, &file->size, file->str);
- if ( !EFI_ERROR(ret) && file->size != size )
- ret = EFI_ABORTED;
- if ( EFI_ERROR(ret) )
- what = L"Read";
- }
+ file->need_to_free = true;
+ file->size = size;
+ handle_file_info(name, file, options);
- if ( FileHandle )
- FileHandle->Close(FileHandle);
+ what = L"Read";
+ ret = FileHandle->Read(FileHandle, &file->size, file->str);
+ if ( !EFI_ERROR(ret) && file->size != size )
+ ret = EFI_ABORTED;
+ if ( EFI_ERROR(ret) )
+ goto fail;
- if ( what )
- {
- PrintErr(what);
- PrintErr(L" failed for ");
- PrintErrMesg(name, ret);
- }
+ FileHandle->Close(FileHandle);
efi_arch_flush_dcache_area(file->ptr, file->size);
return true;
+
+fail:
+ if ( FileHandle )
+ FileHandle->Close(FileHandle);
+
+ PrintErr(what);
+ PrintErr(L" failed for ");
+ PrintErrMesg(name, ret);
+
+ /* not reached */
+ return false;
}
typedef struct __packed {
--
2.43.0On 26.06.2025 15:10, Frediano Ziglio wrote:
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -792,6 +792,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>
> if ( !name )
> PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
> +
> + what = L"Open";
> if ( dir_handle )
> ret = dir_handle->Open(dir_handle, &FileHandle, name,
> EFI_FILE_MODE_READ, 0);
> @@ -800,54 +802,58 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
> if ( file == &cfg && ret == EFI_NOT_FOUND )
> return false;
> if ( EFI_ERROR(ret) )
> - what = L"Open";
> - else
> - ret = FileHandle->SetPosition(FileHandle, -1);
> + goto fail;
> +
> + what = L"Seek";
> + ret = FileHandle->SetPosition(FileHandle, -1);
> if ( EFI_ERROR(ret) )
> - what = what ?: L"Seek";
> - else
> - ret = FileHandle->GetPosition(FileHandle, &size);
> + goto fail;
> +
> + what = L"Get size";
> + ret = FileHandle->GetPosition(FileHandle, &size);
> if ( EFI_ERROR(ret) )
> - what = what ?: L"Get size";
> - else
> - ret = FileHandle->SetPosition(FileHandle, 0);
> + goto fail;
> +
> + what = L"Seek";
> + ret = FileHandle->SetPosition(FileHandle, 0);
> if ( EFI_ERROR(ret) )
> - what = what ?: L"Seek";
> - else
> - {
> - file->addr = min(1UL << (32 + PAGE_SHIFT),
> - HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> - ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> - PFN_UP(size), &file->addr);
> - }
> + goto fail;
> +
> + what = L"Allocation";
> + file->addr = min(1UL << (32 + PAGE_SHIFT),
> + HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> + ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> + PFN_UP(size), &file->addr);
> if ( EFI_ERROR(ret) )
> - what = what ?: L"Allocation";
> - else
> - {
> - file->need_to_free = true;
> - file->size = size;
> - handle_file_info(name, file, options);
> + goto fail;
>
> - ret = FileHandle->Read(FileHandle, &file->size, file->str);
> - if ( !EFI_ERROR(ret) && file->size != size )
> - ret = EFI_ABORTED;
> - if ( EFI_ERROR(ret) )
> - what = L"Read";
> - }
> + file->need_to_free = true;
> + file->size = size;
> + handle_file_info(name, file, options);
>
> - if ( FileHandle )
> - FileHandle->Close(FileHandle);
> + what = L"Read";
> + ret = FileHandle->Read(FileHandle, &file->size, file->str);
> + if ( !EFI_ERROR(ret) && file->size != size )
> + ret = EFI_ABORTED;
> + if ( EFI_ERROR(ret) )
> + goto fail;
>
> - if ( what )
> - {
> - PrintErr(what);
> - PrintErr(L" failed for ");
> - PrintErrMesg(name, ret);
> - }
> + FileHandle->Close(FileHandle);
>
> efi_arch_flush_dcache_area(file->ptr, file->size);
>
> return true;
> +
> +fail:
Nit: Style (see ./CODING_STYLE).
Jan
On Thu, Jun 26, 2025 at 2:31 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.06.2025 15:10, Frediano Ziglio wrote:
> > --- a/xen/common/efi/boot.c
> > +++ b/xen/common/efi/boot.c
> > @@ -792,6 +792,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
> >
> > if ( !name )
> > PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
> > +
> > + what = L"Open";
> > if ( dir_handle )
> > ret = dir_handle->Open(dir_handle, &FileHandle, name,
> > EFI_FILE_MODE_READ, 0);
> > @@ -800,54 +802,58 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
> > if ( file == &cfg && ret == EFI_NOT_FOUND )
> > return false;
> > if ( EFI_ERROR(ret) )
> > - what = L"Open";
> > - else
> > - ret = FileHandle->SetPosition(FileHandle, -1);
> > + goto fail;
> > +
> > + what = L"Seek";
> > + ret = FileHandle->SetPosition(FileHandle, -1);
> > if ( EFI_ERROR(ret) )
> > - what = what ?: L"Seek";
> > - else
> > - ret = FileHandle->GetPosition(FileHandle, &size);
> > + goto fail;
> > +
> > + what = L"Get size";
> > + ret = FileHandle->GetPosition(FileHandle, &size);
> > if ( EFI_ERROR(ret) )
> > - what = what ?: L"Get size";
> > - else
> > - ret = FileHandle->SetPosition(FileHandle, 0);
> > + goto fail;
> > +
> > + what = L"Seek";
> > + ret = FileHandle->SetPosition(FileHandle, 0);
> > if ( EFI_ERROR(ret) )
> > - what = what ?: L"Seek";
> > - else
> > - {
> > - file->addr = min(1UL << (32 + PAGE_SHIFT),
> > - HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> > - ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> > - PFN_UP(size), &file->addr);
> > - }
> > + goto fail;
> > +
> > + what = L"Allocation";
> > + file->addr = min(1UL << (32 + PAGE_SHIFT),
> > + HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> > + ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> > + PFN_UP(size), &file->addr);
> > if ( EFI_ERROR(ret) )
> > - what = what ?: L"Allocation";
> > - else
> > - {
> > - file->need_to_free = true;
> > - file->size = size;
> > - handle_file_info(name, file, options);
> > + goto fail;
> >
> > - ret = FileHandle->Read(FileHandle, &file->size, file->str);
> > - if ( !EFI_ERROR(ret) && file->size != size )
> > - ret = EFI_ABORTED;
> > - if ( EFI_ERROR(ret) )
> > - what = L"Read";
> > - }
> > + file->need_to_free = true;
> > + file->size = size;
> > + handle_file_info(name, file, options);
> >
> > - if ( FileHandle )
> > - FileHandle->Close(FileHandle);
> > + what = L"Read";
> > + ret = FileHandle->Read(FileHandle, &file->size, file->str);
> > + if ( !EFI_ERROR(ret) && file->size != size )
> > + ret = EFI_ABORTED;
> > + if ( EFI_ERROR(ret) )
> > + goto fail;
> >
> > - if ( what )
> > - {
> > - PrintErr(what);
> > - PrintErr(L" failed for ");
> > - PrintErrMesg(name, ret);
> > - }
> > + FileHandle->Close(FileHandle);
> >
> > efi_arch_flush_dcache_area(file->ptr, file->size);
> >
> > return true;
> > +
> > +fail:
>
> Nit: Style (see ./CODING_STYLE).
>
What specifically? I checked the indentation and it's 4 spaces. if-s
are spaced correctly. About labels I didn't find much on CODING_STYLE
so I opened 3/4 files and most of them are indented with no spaces
(they start at column 1).
> Jan
Frediano
On 26.06.2025 15:41, Frediano Ziglio wrote:
> On Thu, Jun 26, 2025 at 2:31 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.06.2025 15:10, Frediano Ziglio wrote:
>>> --- a/xen/common/efi/boot.c
>>> +++ b/xen/common/efi/boot.c
>>> @@ -792,6 +792,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>>
>>> if ( !name )
>>> PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
>>> +
>>> + what = L"Open";
>>> if ( dir_handle )
>>> ret = dir_handle->Open(dir_handle, &FileHandle, name,
>>> EFI_FILE_MODE_READ, 0);
>>> @@ -800,54 +802,58 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>> if ( file == &cfg && ret == EFI_NOT_FOUND )
>>> return false;
>>> if ( EFI_ERROR(ret) )
>>> - what = L"Open";
>>> - else
>>> - ret = FileHandle->SetPosition(FileHandle, -1);
>>> + goto fail;
>>> +
>>> + what = L"Seek";
>>> + ret = FileHandle->SetPosition(FileHandle, -1);
>>> if ( EFI_ERROR(ret) )
>>> - what = what ?: L"Seek";
>>> - else
>>> - ret = FileHandle->GetPosition(FileHandle, &size);
>>> + goto fail;
>>> +
>>> + what = L"Get size";
>>> + ret = FileHandle->GetPosition(FileHandle, &size);
>>> if ( EFI_ERROR(ret) )
>>> - what = what ?: L"Get size";
>>> - else
>>> - ret = FileHandle->SetPosition(FileHandle, 0);
>>> + goto fail;
>>> +
>>> + what = L"Seek";
>>> + ret = FileHandle->SetPosition(FileHandle, 0);
>>> if ( EFI_ERROR(ret) )
>>> - what = what ?: L"Seek";
>>> - else
>>> - {
>>> - file->addr = min(1UL << (32 + PAGE_SHIFT),
>>> - HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
>>> - ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
>>> - PFN_UP(size), &file->addr);
>>> - }
>>> + goto fail;
>>> +
>>> + what = L"Allocation";
>>> + file->addr = min(1UL << (32 + PAGE_SHIFT),
>>> + HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
>>> + ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
>>> + PFN_UP(size), &file->addr);
>>> if ( EFI_ERROR(ret) )
>>> - what = what ?: L"Allocation";
>>> - else
>>> - {
>>> - file->need_to_free = true;
>>> - file->size = size;
>>> - handle_file_info(name, file, options);
>>> + goto fail;
>>>
>>> - ret = FileHandle->Read(FileHandle, &file->size, file->str);
>>> - if ( !EFI_ERROR(ret) && file->size != size )
>>> - ret = EFI_ABORTED;
>>> - if ( EFI_ERROR(ret) )
>>> - what = L"Read";
>>> - }
>>> + file->need_to_free = true;
>>> + file->size = size;
>>> + handle_file_info(name, file, options);
>>>
>>> - if ( FileHandle )
>>> - FileHandle->Close(FileHandle);
>>> + what = L"Read";
>>> + ret = FileHandle->Read(FileHandle, &file->size, file->str);
>>> + if ( !EFI_ERROR(ret) && file->size != size )
>>> + ret = EFI_ABORTED;
>>> + if ( EFI_ERROR(ret) )
>>> + goto fail;
>>>
>>> - if ( what )
>>> - {
>>> - PrintErr(what);
>>> - PrintErr(L" failed for ");
>>> - PrintErrMesg(name, ret);
>>> - }
>>> + FileHandle->Close(FileHandle);
>>>
>>> efi_arch_flush_dcache_area(file->ptr, file->size);
>>>
>>> return true;
>>> +
>>> +fail:
>>
>> Nit: Style (see ./CODING_STYLE).
>>
>
> What specifically? I checked the indentation and it's 4 spaces. if-s
> are spaced correctly. About labels I didn't find much on CODING_STYLE
> so I opened 3/4 files and most of them are indented with no spaces
> (they start at column 1).
You didn't search for the word "label" then, did you? Quote:
'Due to the behavior of GNU diffutils "diff -p", labels should be
indented by at least one blank. Non-case labels inside switch() bodies
are preferred to be indented the same as the block's case labels.'
Jan
On Thu, Jun 26, 2025 at 3:50 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.06.2025 15:41, Frediano Ziglio wrote:
> > On Thu, Jun 26, 2025 at 2:31 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 26.06.2025 15:10, Frediano Ziglio wrote:
> >>> --- a/xen/common/efi/boot.c
> >>> +++ b/xen/common/efi/boot.c
> >>> @@ -792,6 +792,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
> >>>
> >>> if ( !name )
> >>> PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
> >>> +
> >>> + what = L"Open";
> >>> if ( dir_handle )
> >>> ret = dir_handle->Open(dir_handle, &FileHandle, name,
> >>> EFI_FILE_MODE_READ, 0);
> >>> @@ -800,54 +802,58 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
> >>> if ( file == &cfg && ret == EFI_NOT_FOUND )
> >>> return false;
> >>> if ( EFI_ERROR(ret) )
> >>> - what = L"Open";
> >>> - else
> >>> - ret = FileHandle->SetPosition(FileHandle, -1);
> >>> + goto fail;
> >>> +
> >>> + what = L"Seek";
> >>> + ret = FileHandle->SetPosition(FileHandle, -1);
> >>> if ( EFI_ERROR(ret) )
> >>> - what = what ?: L"Seek";
> >>> - else
> >>> - ret = FileHandle->GetPosition(FileHandle, &size);
> >>> + goto fail;
> >>> +
> >>> + what = L"Get size";
> >>> + ret = FileHandle->GetPosition(FileHandle, &size);
> >>> if ( EFI_ERROR(ret) )
> >>> - what = what ?: L"Get size";
> >>> - else
> >>> - ret = FileHandle->SetPosition(FileHandle, 0);
> >>> + goto fail;
> >>> +
> >>> + what = L"Seek";
> >>> + ret = FileHandle->SetPosition(FileHandle, 0);
> >>> if ( EFI_ERROR(ret) )
> >>> - what = what ?: L"Seek";
> >>> - else
> >>> - {
> >>> - file->addr = min(1UL << (32 + PAGE_SHIFT),
> >>> - HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> >>> - ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> >>> - PFN_UP(size), &file->addr);
> >>> - }
> >>> + goto fail;
> >>> +
> >>> + what = L"Allocation";
> >>> + file->addr = min(1UL << (32 + PAGE_SHIFT),
> >>> + HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
> >>> + ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
> >>> + PFN_UP(size), &file->addr);
> >>> if ( EFI_ERROR(ret) )
> >>> - what = what ?: L"Allocation";
> >>> - else
> >>> - {
> >>> - file->need_to_free = true;
> >>> - file->size = size;
> >>> - handle_file_info(name, file, options);
> >>> + goto fail;
> >>>
> >>> - ret = FileHandle->Read(FileHandle, &file->size, file->str);
> >>> - if ( !EFI_ERROR(ret) && file->size != size )
> >>> - ret = EFI_ABORTED;
> >>> - if ( EFI_ERROR(ret) )
> >>> - what = L"Read";
> >>> - }
> >>> + file->need_to_free = true;
> >>> + file->size = size;
> >>> + handle_file_info(name, file, options);
> >>>
> >>> - if ( FileHandle )
> >>> - FileHandle->Close(FileHandle);
> >>> + what = L"Read";
> >>> + ret = FileHandle->Read(FileHandle, &file->size, file->str);
> >>> + if ( !EFI_ERROR(ret) && file->size != size )
> >>> + ret = EFI_ABORTED;
> >>> + if ( EFI_ERROR(ret) )
> >>> + goto fail;
> >>>
> >>> - if ( what )
> >>> - {
> >>> - PrintErr(what);
> >>> - PrintErr(L" failed for ");
> >>> - PrintErrMesg(name, ret);
> >>> - }
> >>> + FileHandle->Close(FileHandle);
> >>>
> >>> efi_arch_flush_dcache_area(file->ptr, file->size);
> >>>
> >>> return true;
> >>> +
> >>> +fail:
> >>
> >> Nit: Style (see ./CODING_STYLE).
> >>
> >
> > What specifically? I checked the indentation and it's 4 spaces. if-s
> > are spaced correctly. About labels I didn't find much on CODING_STYLE
> > so I opened 3/4 files and most of them are indented with no spaces
> > (they start at column 1).
>
> You didn't search for the word "label" then, did you? Quote:
>
I did, I probably mis-typed it.
> 'Due to the behavior of GNU diffutils "diff -p", labels should be
> indented by at least one blank. Non-case labels inside switch() bodies
> are preferred to be indented the same as the block's case labels.'
>
I suppose labels should be indented less than the code they refer to,
so in this case from 1 to 3 spaces. I supposed 2 would be the best
option.
> Jan
>
On 26.06.2025 16:57, Frediano Ziglio wrote:
> On Thu, Jun 26, 2025 at 3:50 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.06.2025 15:41, Frediano Ziglio wrote:
>>> On Thu, Jun 26, 2025 at 2:31 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 26.06.2025 15:10, Frediano Ziglio wrote:
>>>>> --- a/xen/common/efi/boot.c
>>>>> +++ b/xen/common/efi/boot.c
>>>>> @@ -792,6 +792,8 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>>>>
>>>>> if ( !name )
>>>>> PrintErrMesg(L"No filename", EFI_OUT_OF_RESOURCES);
>>>>> +
>>>>> + what = L"Open";
>>>>> if ( dir_handle )
>>>>> ret = dir_handle->Open(dir_handle, &FileHandle, name,
>>>>> EFI_FILE_MODE_READ, 0);
>>>>> @@ -800,54 +802,58 @@ static bool __init read_file(EFI_FILE_HANDLE dir_handle, CHAR16 *name,
>>>>> if ( file == &cfg && ret == EFI_NOT_FOUND )
>>>>> return false;
>>>>> if ( EFI_ERROR(ret) )
>>>>> - what = L"Open";
>>>>> - else
>>>>> - ret = FileHandle->SetPosition(FileHandle, -1);
>>>>> + goto fail;
>>>>> +
>>>>> + what = L"Seek";
>>>>> + ret = FileHandle->SetPosition(FileHandle, -1);
>>>>> if ( EFI_ERROR(ret) )
>>>>> - what = what ?: L"Seek";
>>>>> - else
>>>>> - ret = FileHandle->GetPosition(FileHandle, &size);
>>>>> + goto fail;
>>>>> +
>>>>> + what = L"Get size";
>>>>> + ret = FileHandle->GetPosition(FileHandle, &size);
>>>>> if ( EFI_ERROR(ret) )
>>>>> - what = what ?: L"Get size";
>>>>> - else
>>>>> - ret = FileHandle->SetPosition(FileHandle, 0);
>>>>> + goto fail;
>>>>> +
>>>>> + what = L"Seek";
>>>>> + ret = FileHandle->SetPosition(FileHandle, 0);
>>>>> if ( EFI_ERROR(ret) )
>>>>> - what = what ?: L"Seek";
>>>>> - else
>>>>> - {
>>>>> - file->addr = min(1UL << (32 + PAGE_SHIFT),
>>>>> - HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
>>>>> - ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
>>>>> - PFN_UP(size), &file->addr);
>>>>> - }
>>>>> + goto fail;
>>>>> +
>>>>> + what = L"Allocation";
>>>>> + file->addr = min(1UL << (32 + PAGE_SHIFT),
>>>>> + HYPERVISOR_VIRT_END - DIRECTMAP_VIRT_START);
>>>>> + ret = efi_bs->AllocatePages(AllocateMaxAddress, EfiLoaderData,
>>>>> + PFN_UP(size), &file->addr);
>>>>> if ( EFI_ERROR(ret) )
>>>>> - what = what ?: L"Allocation";
>>>>> - else
>>>>> - {
>>>>> - file->need_to_free = true;
>>>>> - file->size = size;
>>>>> - handle_file_info(name, file, options);
>>>>> + goto fail;
>>>>>
>>>>> - ret = FileHandle->Read(FileHandle, &file->size, file->str);
>>>>> - if ( !EFI_ERROR(ret) && file->size != size )
>>>>> - ret = EFI_ABORTED;
>>>>> - if ( EFI_ERROR(ret) )
>>>>> - what = L"Read";
>>>>> - }
>>>>> + file->need_to_free = true;
>>>>> + file->size = size;
>>>>> + handle_file_info(name, file, options);
>>>>>
>>>>> - if ( FileHandle )
>>>>> - FileHandle->Close(FileHandle);
>>>>> + what = L"Read";
>>>>> + ret = FileHandle->Read(FileHandle, &file->size, file->str);
>>>>> + if ( !EFI_ERROR(ret) && file->size != size )
>>>>> + ret = EFI_ABORTED;
>>>>> + if ( EFI_ERROR(ret) )
>>>>> + goto fail;
>>>>>
>>>>> - if ( what )
>>>>> - {
>>>>> - PrintErr(what);
>>>>> - PrintErr(L" failed for ");
>>>>> - PrintErrMesg(name, ret);
>>>>> - }
>>>>> + FileHandle->Close(FileHandle);
>>>>>
>>>>> efi_arch_flush_dcache_area(file->ptr, file->size);
>>>>>
>>>>> return true;
>>>>> +
>>>>> +fail:
>>>>
>>>> Nit: Style (see ./CODING_STYLE).
>>>>
>>>
>>> What specifically? I checked the indentation and it's 4 spaces. if-s
>>> are spaced correctly. About labels I didn't find much on CODING_STYLE
>>> so I opened 3/4 files and most of them are indented with no spaces
>>> (they start at column 1).
>>
>> You didn't search for the word "label" then, did you? Quote:
>>
>
> I did, I probably mis-typed it.
>
>> 'Due to the behavior of GNU diffutils "diff -p", labels should be
>> indented by at least one blank. Non-case labels inside switch() bodies
>> are preferred to be indented the same as the block's case labels.'
>
> I suppose labels should be indented less than the code they refer to,
> so in this case from 1 to 3 spaces. I supposed 2 would be the best
> option.
Except that I think 1 is what we commonly use (levaing aside the many bad
examples that we still have).
Jan
© 2016 - 2025 Red Hat, Inc.