[v1] user-mode: Prune build dependencies (part 1)

[PATCH 09/14] exec: Drop redundant #ifdeffery

Posted by Philippe Mathieu-Daudé 5 years, 11 months ago

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 exec.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/exec.c b/exec.c
index 7bc9828c5b..f258502966 100644
--- a/exec.c
+++ b/exec.c
@@ -185,10 +185,6 @@ struct DirtyBitmapSnapshot {
     unsigned long dirty[];
 };
 
-#endif
-
-#if !defined(CONFIG_USER_ONLY)
-
 static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
 {
     static unsigned alloc_hint = 16;
-- 
2.21.1

Re: [PATCH 09/14] exec: Drop redundant #ifdeffery

Posted by Alistair Francis 5 years, 11 months ago

On Fri, Mar 13, 2020 at 11:38 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  exec.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 7bc9828c5b..f258502966 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -185,10 +185,6 @@ struct DirtyBitmapSnapshot {
>      unsigned long dirty[];
>  };
>
> -#endif
> -
> -#if !defined(CONFIG_USER_ONLY)
> -
>  static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
>  {
>      static unsigned alloc_hint = 16;
> --
> 2.21.1
>
>

Re: [PATCH 09/14] exec: Drop redundant #ifdeffery

Posted by Richard Henderson 5 years, 10 months ago

On 3/13/20 11:36 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  exec.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 7bc9828c5b..f258502966 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -185,10 +185,6 @@ struct DirtyBitmapSnapshot {
>      unsigned long dirty[];
>  };
>  
> -#endif
> -
> -#if !defined(CONFIG_USER_ONLY)
> -
>  static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
>  {
>      static unsigned alloc_hint = 16;
> 

There's even more than that.  Looking further down,

>     745 #endif
>     746 
>     747 #if !defined(CONFIG_USER_ONLY)

This is the #endif that paired with the one at 190.

Later,

>     988 #if defined(CONFIG_USER_ONLY)
...
>    1000 #else
...
>    1031 #endif
>    1032 
>    1033 #ifndef CONFIG_USER_ONLY

So those three lines are redundant.

Later,

>    1252 #if !defined(CONFIG_USER_ONLY)
...
>    1438 #endif /* defined(CONFIG_USER_ONLY) */
>    1439 
>    1440 #if !defined(CONFIG_USER_ONLY)

Clearly these ifdefs are very hard to follow.  I would thus welcome a split of
this file.

Possibly into exec-common.c (with functions present in both softmmu and
user-only, with ifdefs *inside* functions only), and exec-system.c (with no
/#if.*CONFIG_USER_ONLY/).

But exec.c is over 4000 lines, so if there's another logical split into even
more files, that would be even better.

r~

Re: [PATCH 09/14] exec: Drop redundant #ifdeffery

Posted by Philippe Mathieu-Daudé 5 years, 10 months ago

On 3/15/20 9:39 PM, Richard Henderson wrote:
> On 3/13/20 11:36 AM, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   exec.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index 7bc9828c5b..f258502966 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -185,10 +185,6 @@ struct DirtyBitmapSnapshot {
>>       unsigned long dirty[];
>>   };
>>   
>> -#endif
>> -
>> -#if !defined(CONFIG_USER_ONLY)
>> -
>>   static void phys_map_node_reserve(PhysPageMap *map, unsigned nodes)
>>   {
>>       static unsigned alloc_hint = 16;
>>
> 
> There's even more than that.  Looking further down,
> 
>>      745 #endif
>>      746
>>      747 #if !defined(CONFIG_USER_ONLY)
> 
> This is the #endif that paired with the one at 190.
> 
> Later,
> 
>>      988 #if defined(CONFIG_USER_ONLY)
> ...
>>     1000 #else
> ...
>>     1031 #endif
>>     1032
>>     1033 #ifndef CONFIG_USER_ONLY
> 
> So those three lines are redundant.
> 
> Later,
> 
>>     1252 #if !defined(CONFIG_USER_ONLY)
> ...
>>     1438 #endif /* defined(CONFIG_USER_ONLY) */
>>     1439
>>     1440 #if !defined(CONFIG_USER_ONLY)
> 
> Clearly these ifdefs are very hard to follow.  I would thus welcome a split of
> this file.
> 
> Possibly into exec-common.c (with functions present in both softmmu and
> user-only, with ifdefs *inside* functions only), and exec-system.c (with no
> /#if.*CONFIG_USER_ONLY/).
> 
> But exec.c is over 4000 lines, so if there's another logical split into even
> more files, that would be even better.

OK, I am taking notes for the 5.0 cycle.

We can drop this patch for this series objective, as it is mostly cosmetic.

> 
> 
> r~
>