[PATCH 04/17] configure: clean up handling of CFI option

Paolo Bonzini posted 17 patches 1 year, 1 month ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Thomas Huth <thuth@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Stefano Stabellini <sstabellini@kernel.org>, Anthony Perard <anthony.perard@citrix.com>, Paul Durrant <paul@xen.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Michael Roth <michael.roth@amd.com>, Konstantin Kostiuk <kkostiuk@redhat.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>
[PATCH 04/17] configure: clean up handling of CFI option
Posted by Paolo Bonzini 1 year, 1 month ago
Avoid that --enable-cfi --disable-cfi leaves b_lto set to true.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 configure | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index bce8c57596f..3da46ed202d 100755
--- a/configure
+++ b/configure
@@ -766,11 +766,9 @@ for opt do
   ;;
   --disable-werror) werror="no"
   ;;
-  --enable-cfi)
-      cfi="true";
-      meson_option_add -Db_lto=true
+  --enable-cfi) cfi=true
   ;;
-  --disable-cfi) cfi="false"
+  --disable-cfi) cfi=false
   ;;
   --disable-download) download="disabled"; git_submodules_action=validate;
   ;;
@@ -1845,6 +1843,7 @@ if test "$skip_meson" = no; then
 
   # QEMU options
   test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
+  test "$cfi" != false && meson_option_add "-Db_lto=$cfi"
   test "$docs" != auto && meson_option_add "-Ddocs=$docs"
   test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
   test "$plugins" = yes && meson_option_add "-Dplugins=true"
-- 
2.41.0
Re: [PATCH 04/17] configure: clean up handling of CFI option
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
On 16/10/23 08:31, Paolo Bonzini wrote:
> Avoid that --enable-cfi --disable-cfi leaves b_lto set to true.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   configure | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)


> @@ -1845,6 +1843,7 @@ if test "$skip_meson" = no; then
>   
>     # QEMU options
>     test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
> +  test "$cfi" != false && meson_option_add "-Db_lto=$cfi"

Merge as "-Dcfi=$cfi -Db_lto=$cfi"?

>     test "$docs" != auto && meson_option_add "-Ddocs=$docs"
>     test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
>     test "$plugins" = yes && meson_option_add "-Dplugins=true"
Re: [PATCH 04/17] configure: clean up handling of CFI option
Posted by Paolo Bonzini 1 year, 1 month ago
On 10/16/23 11:22, Philippe Mathieu-Daudé wrote:
> On 16/10/23 08:31, Paolo Bonzini wrote:
>> Avoid that --enable-cfi --disable-cfi leaves b_lto set to true.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   configure | 7 +++----
>>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> 
>> @@ -1845,6 +1843,7 @@ if test "$skip_meson" = no; then
>>     # QEMU options
>>     test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
>> +  test "$cfi" != false && meson_option_add "-Db_lto=$cfi"
> 
> Merge as "-Dcfi=$cfi -Db_lto=$cfi"?

Sure, it also needs a little change to meson_option_add though:

diff --git a/configure b/configure
index 3da46ed202d..fd88ef3fec2 100755
--- a/configure
+++ b/configure
@@ -624,7 +624,10 @@ meson_option_build_array() {
  
  meson_options=
  meson_option_add() {
-  meson_options="$meson_options $(quote_sh "$1")"
+  local arg
+  for arg; do
+    meson_options="$meson_options $(quote_sh "$arg")"
+  done
  }
  meson_option_parse() {
    meson_options="$meson_options $(_meson_option_parse "$@")"
@@ -1842,8 +1845,7 @@ if test "$skip_meson" = no; then
    test "$werror" = yes && meson_option_add -Dwerror=true
  
    # QEMU options
-  test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
-  test "$cfi" != false && meson_option_add "-Db_lto=$cfi"
+  test "$cfi" != false && meson_option_add "-Dcfi=$cfi" "-Db_lto=$cfi"
    test "$docs" != auto && meson_option_add "-Ddocs=$docs"
    test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
    test "$plugins" = yes && meson_option_add "-Dplugins=true"

Ok to squash that in?

Paolo

> 
>>     test "$docs" != auto && meson_option_add "-Ddocs=$docs"
>>     test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add 
>> "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
>>     test "$plugins" = yes && meson_option_add "-Dplugins=true"
> 
> 


Re: [PATCH 04/17] configure: clean up handling of CFI option
Posted by Philippe Mathieu-Daudé 1 year, 1 month ago
On 16/10/23 11:44, Paolo Bonzini wrote:
> On 10/16/23 11:22, Philippe Mathieu-Daudé wrote:
>> On 16/10/23 08:31, Paolo Bonzini wrote:
>>> Avoid that --enable-cfi --disable-cfi leaves b_lto set to true.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>   configure | 7 +++----
>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>
>>
>>> @@ -1845,6 +1843,7 @@ if test "$skip_meson" = no; then
>>>     # QEMU options
>>>     test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
>>> +  test "$cfi" != false && meson_option_add "-Db_lto=$cfi"
>>
>> Merge as "-Dcfi=$cfi -Db_lto=$cfi"?
> 
> Sure, it also needs a little change to meson_option_add though:
> 
> diff --git a/configure b/configure
> index 3da46ed202d..fd88ef3fec2 100755
> --- a/configure
> +++ b/configure
> @@ -624,7 +624,10 @@ meson_option_build_array() {
> 
>   meson_options=
>   meson_option_add() {
> -  meson_options="$meson_options $(quote_sh "$1")"
> +  local arg
> +  for arg; do
> +    meson_options="$meson_options $(quote_sh "$arg")"
> +  done

Oh I didn't notice. If you had said I wouldn't had insisted,
but since you did the change,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   }
>   meson_option_parse() {
>     meson_options="$meson_options $(_meson_option_parse "$@")"
> @@ -1842,8 +1845,7 @@ if test "$skip_meson" = no; then
>     test "$werror" = yes && meson_option_add -Dwerror=true
> 
>     # QEMU options
> -  test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
> -  test "$cfi" != false && meson_option_add "-Db_lto=$cfi"
> +  test "$cfi" != false && meson_option_add "-Dcfi=$cfi" "-Db_lto=$cfi"
>     test "$docs" != auto && meson_option_add "-Ddocs=$docs"
>     test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add 
> "-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
>     test "$plugins" = yes && meson_option_add "-Dplugins=true"
> 
> Ok to squash that in?

Certainly.