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.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
include/qemu/typedefs.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f745d5faf7fd..cd3e369ae01a 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -1,10 +1,9 @@
#ifndef QEMU_TYPEDEFS_H
#define QEMU_TYPEDEFS_H
-/* A load of opaque types so that device init declarations don't have to
- pull in all the real definitions. */
-
-/* Please keep this list in alphabetical order */
+/* First list is for opaque types only, second one for function types.
+ * Please keep both lists in alphabetical order.
+ */
typedef struct AdapterInfo AdapterInfo;
typedef struct AddressSpace AddressSpace;
typedef struct AioContext AioContext;
@@ -96,7 +95,8 @@ typedef struct uWireSlave uWireSlave;
typedef struct VirtIODevice VirtIODevice;
typedef struct Visitor Visitor;
typedef struct node_info NodeInfo;
+
+typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
typedef void SaveStateHandler(QEMUFile *f, void *opaque);
-typedef int LoadStateHandler(QEMUFile *f, void *opaque, int version_id);
#endif /* QEMU_TYPEDEFS_H */
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. thanks -- PMM
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 ? > thanks > -- PMM
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. Later, Juan.
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. thanks -- PMM
* 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? Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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 ? > Dave > > > thanks > > -- PMM > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
* 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. Dave > > Dave > > > > > thanks > > > -- PMM > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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
On 22 June 2017 at 19:08, Thomas Huth <thuth@redhat.com> wrote: > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: >> Could do; I'm just not finding tiny header files with one or >> two entries each that useful. Well, it means that the bulk of code that doesn't care about the types doesn't get its compilation fractionally slowed by having to parse the typedef anyway. In general I think we're drifting towards "have each .c file get fewer things automatically" rather than otherwise (eg more finely focused files rather than stuffing everything into qemu-common.h). > 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? I find function typedefs much more readable than having the function-types inline in function arguments and the like. This is all fairly rapidly heading into bikeshed territory though -- in the final analysis I don't think it matters much what we do. thanks -- PMM
* Peter Maydell (peter.maydell@linaro.org) wrote: > On 22 June 2017 at 19:08, Thomas Huth <thuth@redhat.com> wrote: > > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > >> Could do; I'm just not finding tiny header files with one or > >> two entries each that useful. > > Well, it means that the bulk of code that doesn't care about the > types doesn't get its compilation fractionally slowed by having > to parse the typedef anyway. In general I think we're drifting > towards "have each .c file get fewer things automatically" rather > than otherwise (eg more finely focused files rather than stuffing > everything into qemu-common.h). At the cost of things getting fractionally slower by including lots more tiny headers. I generally agree in the case where there's a useful chunk, but when it's down to one or two definitions I don't see the point. > > 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? > > I find function typedefs much more readable than having the > function-types inline in function arguments and the like. > > This is all fairly rapidly heading into bikeshed territory > though -- in the final analysis I don't think it matters > much what we do. Agreed. Dave > thanks > -- PMM -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Thu, 22 Jun 2017 19:34:58 +0100 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Peter Maydell (peter.maydell@linaro.org) wrote: > > On 22 June 2017 at 19:08, Thomas Huth <thuth@redhat.com> wrote: > > > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > > >> Could do; I'm just not finding tiny header files with one or > > >> two entries each that useful. > > > > Well, it means that the bulk of code that doesn't care about the > > types doesn't get its compilation fractionally slowed by having > > to parse the typedef anyway. In general I think we're drifting > > towards "have each .c file get fewer things automatically" rather > > than otherwise (eg more finely focused files rather than stuffing > > everything into qemu-common.h). > > At the cost of things getting fractionally slower by including lots > more tiny headers. > > I generally agree in the case where there's a useful chunk, > but when it's down to one or two definitions I don't see the point. > > > > 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? > > > > I find function typedefs much more readable than having the > > function-types inline in function arguments and the like. > > > > This is all fairly rapidly heading into bikeshed territory > > though -- in the final analysis I don't think it matters > > much what we do. > > Agreed. > Last question for my own comprehension. I can't think of a case where we would do something like: some_vmsd->load_state_old = some_se->ops->load_state; Does it make sense for VMStateDescription::load_state_old and SaveVMHandlers::load_state to be of the same type ? > Dave > > > thanks > > -- PMM > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Greg Kurz (groug@kaod.org) wrote: > On Thu, 22 Jun 2017 19:34:58 +0100 > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > > > * Peter Maydell (peter.maydell@linaro.org) wrote: > > > On 22 June 2017 at 19:08, Thomas Huth <thuth@redhat.com> wrote: > > > > On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: > > > >> Could do; I'm just not finding tiny header files with one or > > > >> two entries each that useful. > > > > > > Well, it means that the bulk of code that doesn't care about the > > > types doesn't get its compilation fractionally slowed by having > > > to parse the typedef anyway. In general I think we're drifting > > > towards "have each .c file get fewer things automatically" rather > > > than otherwise (eg more finely focused files rather than stuffing > > > everything into qemu-common.h). > > > > At the cost of things getting fractionally slower by including lots > > more tiny headers. > > > > I generally agree in the case where there's a useful chunk, > > but when it's down to one or two definitions I don't see the point. > > > > > > 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? > > > > > > I find function typedefs much more readable than having the > > > function-types inline in function arguments and the like. > > > > > > This is all fairly rapidly heading into bikeshed territory > > > though -- in the final analysis I don't think it matters > > > much what we do. > > > > Agreed. > > > > Last question for my own comprehension. > > I can't think of a case where we would do something like: > > some_vmsd->load_state_old = some_se->ops->load_state; > > Does it make sense for VMStateDescription::load_state_old and SaveVMHandlers::load_state > to be of the same type ? (I think this is what we discussed on irc) There's only a few _old's and they're the same interface as the non-_old's, the only difference is the range of version_id's they're prepared to take. Dave > > Dave > > > > > thanks > > > -- PMM > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Peter Maydell <peter.maydell@linaro.org> writes: > On 22 June 2017 at 19:08, Thomas Huth <thuth@redhat.com> wrote: >> On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: >>> Could do; I'm just not finding tiny header files with one or >>> two entries each that useful. > > Well, it means that the bulk of code that doesn't care about the > types doesn't get its compilation fractionally slowed by having > to parse the typedef anyway. In general I think we're drifting > towards "have each .c file get fewer things automatically" rather > than otherwise (eg more finely focused files rather than stuffing > everything into qemu-common.h). Yes. See also "Our use of #include is undisciplined, and what to do about it" Message-ID: <87wpp4m6n1.fsf@blackfin.pond.sub.org> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03271.html I have some unfinished work towards emptying out qemu-common.h. Need to find the time to finish it. [...]
Markus Armbruster <armbru@redhat.com> wrote: > Peter Maydell <peter.maydell@linaro.org> writes: > >> On 22 June 2017 at 19:08, Thomas Huth <thuth@redhat.com> wrote: >>> On 22.06.2017 19:50, Dr. David Alan Gilbert wrote: >>>> Could do; I'm just not finding tiny header files with one or >>>> two entries each that useful. >> >> Well, it means that the bulk of code that doesn't care about the >> types doesn't get its compilation fractionally slowed by having >> to parse the typedef anyway. In general I think we're drifting >> towards "have each .c file get fewer things automatically" rather >> than otherwise (eg more finely focused files rather than stuffing >> everything into qemu-common.h). > > Yes. See also "Our use of #include is undisciplined, and what to do > about it" > Message-ID: <87wpp4m6n1.fsf@blackfin.pond.sub.org> > https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03271.html > > I have some unfinished work towards emptying out qemu-common.h. Need > to find the time to finish it. > > [...] YES!!!!! Once there, we can also do other cleanups. inclufde/sysemu/sysemu.h have things not related at all. I want to get rid of it on migration, because you know, we do zero emulation there, but there are things like "runstate" that are defined there. I removed on that version lots of migration functionality that were there, just from historical reasons, not because they belong there. Later, Juan.
Peter Maydell <peter.maydell@linaro.org> writes: > 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. The comment is out of date. The header declares many opaque types that have nothing to do with "device init declarations". Suggest to simply delete the comment. The technique to declare opaque types in one place and define them elsewhere is common enough not to require justification. > They're also only used by a very small subset of .c > files, whereas typedefs.h goes everywhere. Yes. Can we find a suitable migration header for them? I don't like tiny header files such as qemu/fnprintf-fn.h.
© 2016 - 2025 Red Hat, Inc.