This is the remainder change to get x86 compiling with -Wwrite-strings.  ARM
still has some cleanup to go.

There are two swamps left.

  1) efi_arch_handle_cmdline().  Swapping name.s for name.cs makes the code
     compile, but only because the underlying union launders the pointer back
     to being mutable.

  2) dom0 cmdline.  There are various string literals which get moved/copied
     around but even with this const-ing, it only compiles because of a
     pointer laundered through strstr().

I think we do want to get Xen to a point where we can compile with
-Wwrite-strings unilaterally active, because it covers a lot of obvious and
simple cases, but that will involve fixing these two swaps and I don't think
we want to just take this patch and call it done.

For ARM, here is a gitlab run from a while back:

  https://gitlab.com/xen-project/people/andyhhp/xen/-/pipelines/931418508

There's one breakage here:

  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";
        |                ^

which which mirrors the x86 side, but others such as:

  drivers/char/arm-uart.c: In function 'dt_uart_init':
  drivers/char/arm-uart.c:81:17: error: assignment discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
     81 |         options = "";
        |                 ^

which will need looking in to.  Please can someone else look into the ARM
side.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/Makefile                          | 2 ++
 xen/arch/x86/dom0_build.c             | 2 +-
 xen/arch/x86/efi/efi-boot.h           | 2 +-
 xen/arch/x86/hvm/dom0_build.c         | 4 ++--
 xen/arch/x86/include/asm/dom0_build.h | 4 ++--
 xen/arch/x86/include/asm/setup.h      | 2 +-
 xen/arch/x86/pv/dom0_build.c          | 2 +-
 xen/arch/x86/setup.c                  | 7 ++++---
 8 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index e8aa66378168..4424460a690a 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -380,6 +380,8 @@ else
 CFLAGS += -fomit-frame-pointer
 endif
 
+CFLAGS += -Wwrite-strings
+
 CFLAGS-$(CONFIG_CC_SPLIT_SECTIONS) += -ffunction-sections -fdata-sections
 
 CFLAGS += -nostdinc -fno-builtin -fno-common
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 9f5300a3efbb..8b1fcc6471d8 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -564,7 +564,7 @@ int __init dom0_setup_permissions(struct domain *d)
 
 int __init construct_dom0(struct domain *d, const module_t *image,
                           unsigned long image_headroom, module_t *initrd,
-                          char *cmdline)
+                          const char *cmdline)
 {
     int rc;
 
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 92f4cfe8bd11..fb64413e6b39 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -324,7 +324,7 @@ static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
         w2s(&name);
     }
     else
-        name.s = "xen";
+        name.cs = "xen";
     place_string(&mbi.cmdline, name.s);
 
     if ( mbi.cmdline )
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index fd2cbf68bc62..a7ae9c3b046e 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -532,7 +532,7 @@ static paddr_t __init find_memory(
 static int __init pvh_load_kernel(struct domain *d, const module_t *image,
                                   unsigned long image_headroom,
                                   module_t *initrd, void *image_base,
-                                  char *cmdline, paddr_t *entry,
+                                  const char *cmdline, paddr_t *entry,
                                   paddr_t *start_info_addr)
 {
     void *image_start = image_base + image_headroom;
@@ -1177,7 +1177,7 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
 int __init dom0_construct_pvh(struct domain *d, const module_t *image,
                               unsigned long image_headroom,
                               module_t *initrd,
-                              char *cmdline)
+                              const char *cmdline)
 {
     paddr_t entry, start_info;
     int rc;
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index a5f8c9e67f68..107c1ff98367 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -16,12 +16,12 @@ int dom0_setup_permissions(struct domain *d);
 int dom0_construct_pv(struct domain *d, const module_t *image,
                       unsigned long image_headroom,
                       module_t *initrd,
-                      char *cmdline);
+                      const char *cmdline);
 
 int dom0_construct_pvh(struct domain *d, const module_t *image,
                        unsigned long image_headroom,
                        module_t *initrd,
-                       char *cmdline);
+                       const char *cmdline);
 
 unsigned long dom0_paging_pages(const struct domain *d,
                                 unsigned long nr_pages);
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index ae0dd3915a61..51fce66607dc 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -35,7 +35,7 @@ int construct_dom0(
     struct domain *d,
     const module_t *kernel, unsigned long kernel_headroom,
     module_t *initrd,
-    char *cmdline);
+    const char *cmdline);
 void setup_io_bitmap(struct domain *d);
 
 unsigned long initial_images_nrpages(nodeid_t node);
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index c99135a5522f..909ee9a899a4 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -358,7 +358,7 @@ int __init dom0_construct_pv(struct domain *d,
                              const module_t *image,
                              unsigned long image_headroom,
                              module_t *initrd,
-                             char *cmdline)
+                             const char *cmdline)
 {
     int i, rc, order, machine;
     bool compatible, compat;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2dbe9857aa60..7cdbe595daf7 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -835,7 +835,7 @@ static bool __init loader_is_grub2(const char *loader_name)
     return (p != NULL) && (p[5] != '0');
 }
 
-static char * __init cmdline_cook(char *p, const char *loader_name)
+static const char *__init cmdline_cook(const char *p, const char *loader_name)
 {
     p = p ? : "";
 
@@ -883,7 +883,7 @@ static struct domain *__init create_dom0(const module_t *image,
         },
     };
     struct domain *d;
-    char *cmdline;
+    const char *cmdline;
     domid_t domid;
 
     if ( opt_dom0_pvh )
@@ -969,7 +969,8 @@ static struct domain *__init create_dom0(const module_t *image,
 void __init noreturn __start_xen(unsigned long mbi_p)
 {
     const char *memmap_type = NULL;
-    char *cmdline, *kextra, *loader;
+    const char *cmdline, *loader;
+    char *kextra;
     void *bsp_stack;
     struct cpu_info *info = get_cpu_info(), *bsp_info;
     unsigned int initrdidx, num_parked = 0;

base-commit: b1c16800e52743d9afd9af62c810f03af16dd942
prerequisite-patch-id: 4c64676f65b22476813ddf3241f1c71c024da970
prerequisite-patch-id: 7a565eba5d6054aef78ea9226a6bdeaeab207405
-- 
2.30.2

On 19.07.2023 13:38, Andrew Cooper wrote:
> This is the remainder change to get x86 compiling with -Wwrite-strings.  ARM
> still has some cleanup to go.
> 
> There are two swamps left.
> 
>   1) efi_arch_handle_cmdline().  Swapping name.s for name.cs makes the code
>      compile, but only because the underlying union launders the pointer back
>      to being mutable.

To address (part of?) this concern, ...

> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -324,7 +324,7 @@ static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
>          w2s(&name);
>      }
>      else
> -        name.s = "xen";
> +        name.cs = "xen";
>      place_string(&mbi.cmdline, name.s);

... how about changing this function invocation to also pass
name.cs? The function already takes const char *, it's just that
there was no "cs" union member back at the time.

Jan