Re: [Qemu-trivial] [Qemu-devel] [PATCH] Separate function types from opaque types in include/qemu/typedefs.h

[Thomas Huth <thuth@redhat.com>]

On 22.06.2017 19:50, Dr. David Alan Gilbert wrote:
> * Greg Kurz (groug@kaod.org) wrote:
>> On Thu, 22 Jun 2017 18:25:55 +0100
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
>>
>>> * Peter Maydell (peter.maydell@linaro.org) wrote:
>>>> On 22 June 2017 at 18:03, Juan Quintela <quintela@redhat.com> wrote:  
>>>>> Greg Kurz <groug@kaod.org> wrote:  
>>>>>> On Thu, 22 Jun 2017 17:14:08 +0100
>>>>>> Peter Maydell <peter.maydell@linaro.org> wrote:
>>>>>>  
>>>>>>> On 22 June 2017 at 17:06, Greg Kurz <groug@kaod.org> wrote:  
>>>>>>>> Function types cannot reside in the same sorted list as opaque types since
>>>>>>>> they may depend on a type which would be defined later.
>>>>>>>>
>>>>>>>> Of course, the same problem could arise if a function type depends on
>>>>>>>> another function type with greater alphabetical order. Hopefully we
>>>>>>>> don't have that at this time.  
>>>>>>>
>>>>>>> The other approach would be to put function types somewhere
>>>>>>> else and leave typedefs.h for the simple 'opaque types
>>>>>>> for structures' that it was started as.
>>>>>>>
>>>>>>> For instance we have include/qemu/fprintf-fn.h as a precedent.
>>>>>>>  
>>>>>>
>>>>>> Indeed, and I'm not quite sure why Juan decided to put these types into
>>>>>> typedefs.h instead of a dedicated header file in include/migration... is
>>>>>> it only because it was the quickest fix ?  
>>>>>
>>>>> All other typedefs were defined there.  I can create a different include
>>>>> file, but I think that is "overengineering", no?  They are typedefs,
>>>>> just not of structs.  But I agree that they are the only ones.  
>>>>
>>>> Well, the comment in the file says "opaque types so that device init
>>>> declarations don't have to pull in all the real definitions", whereas
>>>> the ones you've added aren't opaque types, they are the real
>>>> definitions. They're also only used by a very small subset of .c
>>>> files, whereas typedefs.h goes everywhere.  
>>>
>>> mv fprintf-fn.f   fn-typedefs.h
>>>
>>> move those two defs into that?
>>>
>>
>> Wouldn't it be more appropriate to put them in a dedicated
>> include/migration/handler-fn.h header included by both
>> vmstate.h and register.h ?
> 
> Could do; I'm just not finding tiny header files with one or
> two entries each that useful.

Do we really need these function typedefs at all? IMHO it's quite ugly
to hide such things in a typedef unless it is really necessary (and in
this case, it does not seem to be really necessary since it is only used
in a few places). So what about simply removing the typedefs in this case?

 Thomas