On 22.09.2021 16:13, Luca Fancellu wrote:
> +static unsigned int __init allocate_dom0less_file(EFI_FILE_HANDLE dir_handle,
> + const char *name,
> + unsigned int name_len)
> +{
> + dom0less_module_name* file_name;
> + union string module_name;
> + unsigned int ret_idx;
> +
> + /*
> + * Check if there is any space left for a domU module, the variable
> + * dom0less_modules_available is updated each time we use read_file(...)
> + * successfully.
> + */
> + if ( !dom0less_modules_available )
> + blexit(L"No space left for domU modules");
> +
> + module_name.s = (char*) name;
Unfortunately there are too many style issues in these Arm additions to
really enumerate; I'd like to ask that you go through yourself with
./CODING_STYLE, surrounding code, and review comments on earlier patches
of yours in mind. This cast stands out, though: I'm pretty sure you were
told before that casts are often dangerous and hence should be avoided
whenever (easily) possible. There was a prior case where union string
was used in a similar way, not all that long ago. Hence why it now has
a "const char *" member. (That's still somewhat risky, but imo way
better than a cast.)
> @@ -1361,12 +1360,21 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE
> *SystemTable)
> efi_bs->FreePages(cfg.addr, PFN_UP(cfg.size));
> cfg.addr = 0;
>
> - dir_handle->Close(dir_handle);
> -
> if ( gop && !base_video )
> gop_mode = efi_find_gop_mode(gop, cols, rows, depth);
> }
>
> + /*
> + * Check if a proper configuration is provided to start Xen:
> + * - Dom0 specified (minimum required)
> + * - Dom0 and DomU(s) specified
> + * - DomU(s) specified
> + */
> + if ( !efi_arch_check_dom0less_boot(dir_handle) && !kernel.addr )
> + blexit(L"No Dom0 kernel image specified.");
> +
> + dir_handle->Close(dir_handle);
So far I was under the impression that handles and alike need closing
before calling Exit(), to prevent resource leaks. While I will admit
that likely there are more (pre-existing) affected paths, I think that
- if that understanding of mine is correct - it would be nice to avoid
adding yet more instances.
Jan
