On 21/11/2023 8:40 am, Jan Beulich wrote: > On 20.11.2023 23:49, Andrew Cooper wrote: >> GCC complains: >> >> In file included from arch/arm/efi/boot.c:700: >> arch/arm/efi/efi-boot.h: In function 'efi_arch_handle_cmdline': >> arch/arm/efi/efi-boot.h:482:16: error: assignment discards 'const' >> qualifier from pointer target type [-Werror=discarded-qualifiers] >> 482 | name.s = "xen"; >> | ^ >> >> There's no easy option. .rodata is really read-only, so the fact Xen doesn't >> crash means these strings aren't written to. > And the consuming sites confirm this being the case. Hence ... > >> Lie to the compiler using a union. > ... to at least slightly limit the lying, how about ... > >> --- a/xen/arch/arm/efi/efi-boot.h >> +++ b/xen/arch/arm/efi/efi-boot.h >> @@ -479,7 +479,7 @@ static void __init efi_arch_handle_cmdline(CHAR16 >> *image_name, >> w2s(&name); >> } >> else >> - name.s = "xen"; >> + name.cs = "xen"; /* TODO, find a better way of doing this. */ >> >> prop_len = 0; >> prop_len += snprintf(buf + prop_len, > ... you also switch to using name.cs down below here and ... > >> --- a/xen/arch/x86/efi/efi-boot.h >> +++ b/xen/arch/x86/efi/efi-boot.h >> @@ -324,7 +324,8 @@ static void __init efi_arch_handle_cmdline(CHAR16 >> *image_name, >> w2s(&name); >> } >> else >> - name.s = "xen"; >> + name.cs = "xen"; /* TODO, find a better way of doing this. */ >> + >> place_string(&mbi.cmdline, name.s); > ... here? > > An alternative would be to introduce 'char xen[4] = "xen";' in both > cases, and use them instead of plain string literals.
They'd have to be static, or dynamically allocated or they'll end up out of scope, wont they? I have to admit I find this logic very hard to follow. Furthermore, I note: mbi.boot_loader_name = (long)"EFI"; which is exactly the kind of pointer which is liable to end up being edited via kextra in the other patch. ~Andrew
