include path cleanup

[Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Michael S. Tsirkin 7 years, 5 months ago

we just need a struct name, let's add a forward
declaration instead of an include.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/s390x/sclp.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index f9db243..6e65150 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -16,7 +16,8 @@
 
 #include "hw/sysbus.h"
 #include "hw/qdev.h"
-#include "target/s390x/cpu-qom.h"
+
+typedef struct CPUS390XState CPUS390XState;
 
 #define SCLP_CMD_CODE_MASK                      0xffff00ff
 
-- 
MST

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Thomas Huth 7 years, 5 months ago

On 03.05.2018 21:50, Michael S. Tsirkin wrote:
> we just need a struct name, let's add a forward
> declaration instead of an include.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/hw/s390x/sclp.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index f9db243..6e65150 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -16,7 +16,8 @@
>  
>  #include "hw/sysbus.h"
>  #include "hw/qdev.h"
> -#include "target/s390x/cpu-qom.h"
> +
> +typedef struct CPUS390XState CPUS390XState;

IIRC you have to use include/qemu/typedefs.h instead to avoid trouble
with older versions of GCC.

 Thomas

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Cornelia Huck 7 years, 5 months ago

On Fri, 4 May 2018 02:24:12 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 03.05.2018 21:50, Michael S. Tsirkin wrote:
> > we just need a struct name, let's add a forward
> > declaration instead of an include.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/hw/s390x/sclp.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> > index f9db243..6e65150 100644
> > --- a/include/hw/s390x/sclp.h
> > +++ b/include/hw/s390x/sclp.h
> > @@ -16,7 +16,8 @@
> >  
> >  #include "hw/sysbus.h"
> >  #include "hw/qdev.h"
> > -#include "target/s390x/cpu-qom.h"
> > +
> > +typedef struct CPUS390XState CPUS390XState;  
> 
> IIRC you have to use include/qemu/typedefs.h instead to avoid trouble
> with older versions of GCC.

Hm, I'm wondering why we do the typedef in cpu-qom.h, while other
architectures do it in their cpu.h.

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Thomas Huth 7 years, 5 months ago

On 08.05.2018 15:23, Cornelia Huck wrote:
> On Fri, 4 May 2018 02:24:12 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 03.05.2018 21:50, Michael S. Tsirkin wrote:
>>> we just need a struct name, let's add a forward
>>> declaration instead of an include.
>>>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>  include/hw/s390x/sclp.h | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>>> index f9db243..6e65150 100644
>>> --- a/include/hw/s390x/sclp.h
>>> +++ b/include/hw/s390x/sclp.h
>>> @@ -16,7 +16,8 @@
>>>  
>>>  #include "hw/sysbus.h"
>>>  #include "hw/qdev.h"
>>> -#include "target/s390x/cpu-qom.h"
>>> +
>>> +typedef struct CPUS390XState CPUS390XState;  
>>
>> IIRC you have to use include/qemu/typedefs.h instead to avoid trouble
>> with older versions of GCC.
> 
> Hm, I'm wondering why we do the typedef in cpu-qom.h, while other
> architectures do it in their cpu.h.

See:

commit ef2974cc270d51959ce90df6b4d4d41635d7a603
Author: David Hildenbrand <david@redhat.com>
Date:   Wed Sep 13 15:24:02 2017 +0200

    target/s390x: move some s390x typedefs to cpu-qom.h
    
    This allows us to drop inclusion of cpu_models.h in cpu-qom.h, and
    prepares for using cpu-qom.h as a s390 specific version of typedefs.h
    
    Signed-off-by: David Hildenbrand <david@redhat.com>
    Message-Id: <20170913132417.24384-8-david@redhat.com>
    Reviewed-by: Thomas Huth <thuth@redhat.com>
    Signed-off-by: Cornelia Huck <cohuck@redhat.com>

 Thomas

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Cornelia Huck 7 years, 5 months ago

On Tue, 8 May 2018 15:38:03 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 08.05.2018 15:23, Cornelia Huck wrote:
> > On Fri, 4 May 2018 02:24:12 +0200
> > Thomas Huth <thuth@redhat.com> wrote:
> >   
> >> On 03.05.2018 21:50, Michael S. Tsirkin wrote:  
> >>> we just need a struct name, let's add a forward
> >>> declaration instead of an include.
> >>>
> >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>> ---
> >>>  include/hw/s390x/sclp.h | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> >>> index f9db243..6e65150 100644
> >>> --- a/include/hw/s390x/sclp.h
> >>> +++ b/include/hw/s390x/sclp.h
> >>> @@ -16,7 +16,8 @@
> >>>  
> >>>  #include "hw/sysbus.h"
> >>>  #include "hw/qdev.h"
> >>> -#include "target/s390x/cpu-qom.h"
> >>> +
> >>> +typedef struct CPUS390XState CPUS390XState;    
> >>
> >> IIRC you have to use include/qemu/typedefs.h instead to avoid trouble
> >> with older versions of GCC.  
> > 
> > Hm, I'm wondering why we do the typedef in cpu-qom.h, while other
> > architectures do it in their cpu.h.  
> 
> See:
> 
> commit ef2974cc270d51959ce90df6b4d4d41635d7a603
> Author: David Hildenbrand <david@redhat.com>
> Date:   Wed Sep 13 15:24:02 2017 +0200
> 
>     target/s390x: move some s390x typedefs to cpu-qom.h
>     
>     This allows us to drop inclusion of cpu_models.h in cpu-qom.h, and
>     prepares for using cpu-qom.h as a s390 specific version of typedefs.h
>     
>     Signed-off-by: David Hildenbrand <david@redhat.com>
>     Message-Id: <20170913132417.24384-8-david@redhat.com>
>     Reviewed-by: Thomas Huth <thuth@redhat.com>
>     Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> 
>  Thomas
> 
> 

Gargh, this is all very confusing...

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Thomas Huth 7 years, 5 months ago

On 08.05.2018 15:45, Cornelia Huck wrote:
> On Tue, 8 May 2018 15:38:03 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 08.05.2018 15:23, Cornelia Huck wrote:
>>> On Fri, 4 May 2018 02:24:12 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>   
>>>> On 03.05.2018 21:50, Michael S. Tsirkin wrote:  
>>>>> we just need a struct name, let's add a forward
>>>>> declaration instead of an include.
>>>>>
>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>> ---
>>>>>  include/hw/s390x/sclp.h | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>>>>> index f9db243..6e65150 100644
>>>>> --- a/include/hw/s390x/sclp.h
>>>>> +++ b/include/hw/s390x/sclp.h
>>>>> @@ -16,7 +16,8 @@
>>>>>  
>>>>>  #include "hw/sysbus.h"
>>>>>  #include "hw/qdev.h"
>>>>> -#include "target/s390x/cpu-qom.h"
>>>>> +
>>>>> +typedef struct CPUS390XState CPUS390XState;    
>>>>
>>>> IIRC you have to use include/qemu/typedefs.h instead to avoid trouble
>>>> with older versions of GCC.  
>>>
>>> Hm, I'm wondering why we do the typedef in cpu-qom.h, while other
>>> architectures do it in their cpu.h.  
>>
>> See:
>>
>> commit ef2974cc270d51959ce90df6b4d4d41635d7a603
>> Author: David Hildenbrand <david@redhat.com>
>> Date:   Wed Sep 13 15:24:02 2017 +0200
>>
>>     target/s390x: move some s390x typedefs to cpu-qom.h
>>     
>>     This allows us to drop inclusion of cpu_models.h in cpu-qom.h, and
>>     prepares for using cpu-qom.h as a s390 specific version of typedefs.h
>>     
>>     Signed-off-by: David Hildenbrand <david@redhat.com>
>>     Message-Id: <20170913132417.24384-8-david@redhat.com>
>>     Reviewed-by: Thomas Huth <thuth@redhat.com>
>>     Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>
>>  Thomas
> 
> Gargh, this is all very confusing...

If you'd ask me, I'd say we should get rid of the typedefs and do it the
Linux kernel way and enforce using "struct xyz" everywhere, then you
also do not have this problem with typedefs.h anymore ... but well, so
far it seems as I'm still part of a minority with this opinion here.

 Thomas

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

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

On 05/08/2018 10:50 AM, Thomas Huth wrote:
> On 08.05.2018 15:45, Cornelia Huck wrote:
>> On Tue, 8 May 2018 15:38:03 +0200
>> Thomas Huth <thuth@redhat.com> wrote:
>>
>>> On 08.05.2018 15:23, Cornelia Huck wrote:
>>>> On Fri, 4 May 2018 02:24:12 +0200
>>>> Thomas Huth <thuth@redhat.com> wrote:
>>>>   
>>>>> On 03.05.2018 21:50, Michael S. Tsirkin wrote:  
>>>>>> we just need a struct name, let's add a forward
>>>>>> declaration instead of an include.
>>>>>>
>>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>> ---
>>>>>>  include/hw/s390x/sclp.h | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
>>>>>> index f9db243..6e65150 100644
>>>>>> --- a/include/hw/s390x/sclp.h
>>>>>> +++ b/include/hw/s390x/sclp.h
>>>>>> @@ -16,7 +16,8 @@
>>>>>>  
>>>>>>  #include "hw/sysbus.h"
>>>>>>  #include "hw/qdev.h"
>>>>>> -#include "target/s390x/cpu-qom.h"
>>>>>> +
>>>>>> +typedef struct CPUS390XState CPUS390XState;    
>>>>>
>>>>> IIRC you have to use include/qemu/typedefs.h instead to avoid trouble
>>>>> with older versions of GCC.  
>>>>
>>>> Hm, I'm wondering why we do the typedef in cpu-qom.h, while other
>>>> architectures do it in their cpu.h.  
>>>
>>> See:
>>>
>>> commit ef2974cc270d51959ce90df6b4d4d41635d7a603
>>> Author: David Hildenbrand <david@redhat.com>
>>> Date:   Wed Sep 13 15:24:02 2017 +0200
>>>
>>>     target/s390x: move some s390x typedefs to cpu-qom.h
>>>     
>>>     This allows us to drop inclusion of cpu_models.h in cpu-qom.h, and
>>>     prepares for using cpu-qom.h as a s390 specific version of typedefs.h
>>>     
>>>     Signed-off-by: David Hildenbrand <david@redhat.com>
>>>     Message-Id: <20170913132417.24384-8-david@redhat.com>
>>>     Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>     Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>>
>>>  Thomas
>>
>> Gargh, this is all very confusing...
> 
> If you'd ask me, I'd say we should get rid of the typedefs and do it the
> Linux kernel way and enforce using "struct xyz" everywhere, then you
> also do not have this problem with typedefs.h anymore ... but well, so
> far it seems as I'm still part of a minority with this opinion here.

Maybe not getting rid of the typedefs, but I agree with removing typedefs.h.

Re: [Qemu-devel] [PATCH 04/67] s390x: drop an unused include

Posted by Thomas Huth 7 years, 5 months ago

On 08.05.2018 16:06, Philippe Mathieu-Daudé wrote:
> On 05/08/2018 10:50 AM, Thomas Huth wrote:
>> On 08.05.2018 15:45, Cornelia Huck wrote:
>>> On Tue, 8 May 2018 15:38:03 +0200
>>> Thomas Huth <thuth@redhat.com> wrote:
>>>
>>>> On 08.05.2018 15:23, Cornelia Huck wrote:
[...]
>>>>> Hm, I'm wondering why we do the typedef in cpu-qom.h, while other
>>>>> architectures do it in their cpu.h.  
>>>>
>>>> See:
>>>>
>>>> commit ef2974cc270d51959ce90df6b4d4d41635d7a603
>>>> Author: David Hildenbrand <david@redhat.com>
>>>> Date:   Wed Sep 13 15:24:02 2017 +0200
>>>>
>>>>     target/s390x: move some s390x typedefs to cpu-qom.h
>>>>     
>>>>     This allows us to drop inclusion of cpu_models.h in cpu-qom.h, and
>>>>     prepares for using cpu-qom.h as a s390 specific version of typedefs.h
>>>>     
>>>>     Signed-off-by: David Hildenbrand <david@redhat.com>
>>>>     Message-Id: <20170913132417.24384-8-david@redhat.com>
>>>>     Reviewed-by: Thomas Huth <thuth@redhat.com>
>>>>     Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>>>
>>>>  Thomas
>>>
>>> Gargh, this is all very confusing...
>>
>> If you'd ask me, I'd say we should get rid of the typedefs and do it the
>> Linux kernel way and enforce using "struct xyz" everywhere, then you
>> also do not have this problem with typedefs.h anymore ... but well, so
>> far it seems as I'm still part of a minority with this opinion here.
> 
> Maybe not getting rid of the typedefs, but I agree with removing typedefs.h.

But you need a way to do forward declarations ... thus you need
something like typedefs.h as long as you use typedefs. Otherwise older
versions of GCC will choke on multiple "typedef struct XyZ XyZ" statements.

 Thomas