On Fri, Nov 20, 2020 at 06:29:16AM +0100, Markus Armbruster wrote: > Eduardo Habkost <ehabk...@redhat.com> writes: > > > On Thu, Nov 19, 2020 at 11:24:52AM +0100, Markus Armbruster wrote: > >> Marc-André Lureau <marcandre.lur...@gmail.com> writes: > >> > >> > On Tue, Nov 17, 2020 at 6:42 PM Eduardo Habkost <ehabk...@redhat.com> > >> > wrote: > >> > > >> >> On Tue, Nov 17, 2020 at 12:37:56PM +0400, Marc-André Lureau wrote: > >> >> > On Tue, Nov 17, 2020 at 2:43 AM Eduardo Habkost <ehabk...@redhat.com> > >> >> wrote: > >> >> > > >> >> > > Provide a separate QNumValue type that can be used for QNum value > >> >> > > literals without the referencing counting and memory allocation > >> >> > > features provided by QObject. > >> >> > > > >> >> > > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > >> >> > > --- > >> >> > > Changes v1 -> v2: > >> >> > > * Fix "make check" failure, by updating check-qnum unit test to > >> >> > > use the new struct fields > >> >> > > --- > >> >> > > include/qapi/qmp/qnum.h | 40 +++++++++++++++++++-- > >> >> > > qobject/qnum.c | 78 > >> >> > > ++++++++++++++++++++--------------------- > >> >> > > tests/check-qnum.c | 14 ++++---- > >> >> > > 3 files changed, 84 insertions(+), 48 deletions(-) > >> >> > > > >> >> > > diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h > >> >> > > index 55c27b1c24..62fbdfda68 100644 > >> >> > > --- a/include/qapi/qmp/qnum.h > >> >> > > +++ b/include/qapi/qmp/qnum.h > >> >> > > @@ -46,20 +46,56 @@ typedef enum { > >> >> > > * in range: qnum_get_try_int() / qnum_get_try_uint() check range > >> >> > > and > >> >> > > * convert under the hood. > >> >> > > */ > >> >> > > -struct QNum { > >> >> > > - struct QObjectBase_ base; > >> >> > > + > >> >> > > +/** > >> >> > > + * struct QNumValue: the value of a QNum > >> >> > > + * > >> >> > > + * QNumValue literals can be constructed using the `QNUM_VAL_INT`, > >> >> > > + * `QNUM_VAL_UINT`, and `QNUM_VAL_DOUBLE` macros. > >> >> > > + */ > >> >> > > +typedef struct QNumValue { > >> >> > > + /* private: */ > >> > >> Do we care? > > > > Are you asking if we want to make them private, or if we want to > > document them as private (assuming they are). > > > > The answer to the latter is yes ("private:" is an indication to > > kernel-doc). The answer to the former seems to be "no", based on > > your other comments. > > > > Or maybe `kind` should be public and `u` should be private? > > You're factoring out a part of struct QNum into new struct QNumValue. > struct QNum is not private before the patch. I see no need to start > making it or parts of it private now.
I don't want to change the rules, just to document the existing implicit rules. If you say QNum.u was never private, I won't argue. > > When the structure of a data type is to be kept away from its users, I > prefer to keep it out of the public header, so the compiler enforces the > encapsulation. I prefer that too, except that it is impossible when users of the API need the compiler to know the struct size. -- Eduardo