On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > * Greg Kurz (gr...@kaod.org) wrote: >> On Thu, 22 Jun 2017 18:25:55 +0100 >> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote: >> >>> * Peter Maydell (peter.mayd...@linaro.org) wrote: >>>> On 22 June 2017 at 18:03, Juan Quintela <quint...@redhat.com> wrote: >>>>> Greg Kurz <gr...@kaod.org> wrote: >>>>>> On Thu, 22 Jun 2017 17:14:08 +0100 >>>>>> Peter Maydell <peter.mayd...@linaro.org> wrote: >>>>>> >>>>>>> On 22 June 2017 at 17:06, Greg Kurz <gr...@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
