From b066ae906b7e44c50c5233129db921081aa2e4b0 Mon Sep 17 00:00:00 2001
From: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
Date: Mon, 14 Mar 2022 12:56:40 +0100
Subject: [PATCH] cmake: fix improper use of separate_arguments

The use of `separate_arguments()` in extra_flags.cmake and
unittest.cmake prevents the use of proper CMake lists.

A CMake list can be specified on command line as:
`-DMY_LIST="itemA;itemB;itemC"`

Users are expected to be able to provide extra compile flags using CMake
lists, like `-DEXTRA_CFLAGS="--flag1;--flag2"` but also using space
separated lists, like: `-DEXTRA_CFLAGS="--flag1 --flag2"`.

However, the way FLAGS was passed to separate_arguments() would result
in a list being treated as extra arguments to the function and thus
result in a CMake error.

To allow the use of CMake lists, the length of the argument as a CMake
list is tested. This ensures that when the argument contains a
single `;` then the length is >1 and no conversion is needed.

This allows a user to freely choose between the two forms:
- `-DFLAGS="--flag1 --flag2"`
- `-DFLAGS="--flag1;--flag2"`

or when passing flags which themselves contains spaces, like:
- `-DFLAGS="--flag1 \"val1 val2\""`
- `-DFLAGS="--flag1;val1 val2"`

Note that the extra escaped qouting is no longer needed for proper CMake
lists as the `;` itself informs the system that spaces are to be kept as
spaces.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
---
 cmake/extra_flags.cmake      | 17 ++++++++++++-----
 cmake/modules/unittest.cmake | 17 ++++++++++++-----
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/cmake/extra_flags.cmake b/cmake/extra_flags.cmake
index fa03a87fcc9..2e41720991d 100644
--- a/cmake/extra_flags.cmake
+++ b/cmake/extra_flags.cmake
@@ -1,10 +1,17 @@
 # SPDX-License-Identifier: Apache-2.0
 
-separate_arguments(EXTRA_CPPFLAGS_AS_LIST UNIX_COMMAND ${EXTRA_CPPFLAGS})
-separate_arguments(EXTRA_LDFLAGS_AS_LIST UNIX_COMMAND  ${EXTRA_LDFLAGS})
-separate_arguments(EXTRA_CFLAGS_AS_LIST   UNIX_COMMAND ${EXTRA_CFLAGS})
-separate_arguments(EXTRA_CXXFLAGS_AS_LIST UNIX_COMMAND ${EXTRA_CXXFLAGS})
-separate_arguments(EXTRA_AFLAGS_AS_LIST   UNIX_COMMAND ${EXTRA_AFLAGS})
+foreach(extra_flags EXTRA_CPPFLAGS EXTRA_LDFLAGS EXTRA_CFLAGS EXTRA_CXXFLAGS EXTRA_AFLAGS)
+  list(LENGTH ${extra_flags} flags_length)
+  if(flags_length LESS_EQUAL 1)
+    # A length of zero means no argument.
+    # A length of one means a single argument or a space separated list was provided.
+    # In both cases, it is safe to do a separate_arguments on the argument.
+    separate_arguments(${extra_flags}_AS_LIST UNIX_COMMAND ${${extra_flags}})
+  else()
+    # Already a proper list, no conversion needed.
+    set(${extra_flags}_AS_LIST "${${extra_flags}}")
+  endif()
+endforeach()
 
 if(EXTRA_CPPFLAGS)
   zephyr_compile_options(${EXTRA_CPPFLAGS_AS_LIST})
diff --git a/cmake/modules/unittest.cmake b/cmake/modules/unittest.cmake
index 1b33478c2e2..7ff528605fa 100644
--- a/cmake/modules/unittest.cmake
+++ b/cmake/modules/unittest.cmake
@@ -14,11 +14,18 @@ include(kconfig)
 #   SOURCES: list of source files, default main.c
 #   INCLUDE: list of additional include paths relative to ZEPHYR_BASE
 
-separate_arguments(  EXTRA_CFLAGS_AS_LIST UNIX_COMMAND ${EXTRA_CFLAGS})
-separate_arguments(  EXTRA_AFLAGS_AS_LIST UNIX_COMMAND ${EXTRA_AFLAGS})
-separate_arguments(EXTRA_CPPFLAGS_AS_LIST UNIX_COMMAND ${EXTRA_CPPFLAGS})
-separate_arguments(EXTRA_CXXFLAGS_AS_LIST UNIX_COMMAND ${EXTRA_CXXFLAGS})
-separate_arguments(EXTRA_LDFLAGS_AS_LIST UNIX_COMMAND  ${EXTRA_LDFLAGS})
+foreach(extra_flags EXTRA_CPPFLAGS EXTRA_LDFLAGS EXTRA_CFLAGS EXTRA_CXXFLAGS EXTRA_AFLAGS)
+  list(LENGTH ${extra_flags} flags_length)
+  if(flags_length LESS_EQUAL 1)
+    # A length of zero means no argument.
+    # A length of one means a single argument or a space separated list was provided.
+    # In both cases, it is safe to do a separate_arguments on the argument.
+    separate_arguments(${extra_flags}_AS_LIST UNIX_COMMAND ${${extra_flags}})
+  else()
+    # Already a proper list, no conversion needed.
+    set(${extra_flags}_AS_LIST "${${extra_flags}}")
+  endif()
+endforeach()
 
 set(ENV_ZEPHYR_BASE $ENV{ZEPHYR_BASE})
 # This add support for old style boilerplate include.