Re: [Qemu-devel] [PATCH 04/14] qlit: remove needless type cast

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Hi
>
> On Fri, Aug 25, 2017 at 8:20 AM Markus Armbruster <address@hidden> wrote:
>
>> Marc-André Lureau <address@hidden> writes:
>>
>> > And misc code style fix.
>> >
>> > Signed-off-by: Marc-André Lureau <address@hidden>
>> > ---
>> >  include/qapi/qmp/qlit.h | 8 ++++----
>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
>> > index f36bca4554..1e9696988a 100644
>> > --- a/include/qapi/qmp/qlit.h
>> > +++ b/include/qapi/qmp/qlit.h
>> > @@ -36,13 +36,13 @@ struct QLitDictEntry {
>> >  };
>> >
>> >  #define QLIT_QNUM(val) \
>> > -    (QLitObject){.type = QTYPE_QNUM, .value.qnum = (val)}
>> > +    { .type = QTYPE_QNUM, .value.qnum = (val) }
>> >  #define QLIT_QSTR(val) \
>> > -    (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
>> > +    { .type = QTYPE_QSTRING, .value.qstr = (val) }
>> >  #define QLIT_QDICT(val) \
>> > -    (QLitObject){.type = QTYPE_QDICT, .value.qdict = (val)}
>> > +    { .type = QTYPE_QDICT, .value.qdict = (val) }
>> >  #define QLIT_QLIST(val) \
>> > -    (QLitObject){.type = QTYPE_QLIST, .value.qlist = (val)}
>> > +    { .type = QTYPE_QLIST, .value.qlist = (val) }
>> >
>> >  int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);
>>
>> Technically, this isn't a type cast, it's a compound literal.  A
>> compound literal is "an unnamed object whose value is given by the
>> initializer list" (C99 §6.5.2.5).  The standard then cautions "that this
>> differs from a cast expression" (ibid).
>
> You are right, but actually the patch helps with another issue if we keep
> the compound type:
>
> const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>             QLIT_QNULL,
>             {}
>         }));
>
> qmp-introspect.c:17:63: error: initializer element is not constant
>  const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>                                                                ^
> /home/elmarco/src/qemu/include/qapi/qmp/qlit.h:50:57: note: in definition
> of macro ‘QLIT_QLIST’
>      (QLitObject) { .type = QTYPE_QLIST, .value.qlist = (val) }
>                                                          ^~~
> qmp-introspect.c:17:63: note: (near initialization for ‘(anonymous).value’)
>  const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>                                                                ^
> /home/elmarco/src/qemu/include/qapi/qmp/qlit.h:50:57: note: in definition
> of macro ‘QLIT_QLIST’
>      (QLitObject) { .type = QTYPE_QLIST, .value.qlist = (val) }
>                                                          ^~~
> Any idea how to fix that?

This one:

[...]
>> The original author's choice of compound literals is clearly
>> intentional.  I'd prefer to respect it unless there's a compelling
>> reason not to.

Rewrite your commit message so it explains your *actual* reason for
converting the macro from compound literal to initializer.