Patchwork [02/17] qidl: add qc definitions

login
register
mail settings
Submitter Michael Roth
Date June 5, 2012, 1 a.m.
Message ID <1338858018-17189-3-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/162898/
State New
Headers show

Comments

Michael Roth - June 5, 2012, 1 a.m.
Define away the annotations so we can still compile.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qc.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
 create mode 100644 qapi/qc.h
Kevin Wolf - June 5, 2012, 9:25 a.m.
Am 05.06.2012 03:00, schrieb Michael Roth:
> Define away the annotations so we can still compile.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qapi/qc.h |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>  create mode 100644 qapi/qc.h
> 
> diff --git a/qapi/qc.h b/qapi/qc.h
> new file mode 100644
> index 0000000..3b3a8b9
> --- /dev/null
> +++ b/qapi/qc.h
> @@ -0,0 +1,11 @@
> +#ifndef QC_H
> +#define QC_H
> +
> +#define qc_declaration
> +#define _immutable
> +#define _derived
> +#define _broken
> +#define _version(x)
> +#define _size_is(x)
> +
> +#endif

I believe there are still ways to improve the documentation here. :-)

At the very least there should be a reference to the documentation of
patch 1.

Kevin
Jan Kiszka - June 5, 2012, 10:35 a.m.
On 2012-06-05 03:00, Michael Roth wrote:
> Define away the annotations so we can still compile.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qapi/qc.h |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>  create mode 100644 qapi/qc.h
> 
> diff --git a/qapi/qc.h b/qapi/qc.h
> new file mode 100644
> index 0000000..3b3a8b9
> --- /dev/null
> +++ b/qapi/qc.h
> @@ -0,0 +1,11 @@
> +#ifndef QC_H
> +#define QC_H
> +
> +#define qc_declaration
> +#define _immutable
> +#define _derived
> +#define _broken
> +#define _version(x)
> +#define _size_is(x)
> +
> +#endif

These tags require a different prefix than the reserved '_'.

Jan
Anthony Liguori - June 5, 2012, 11:12 a.m.
On 06/05/2012 06:35 PM, Jan Kiszka wrote:
> On 2012-06-05 03:00, Michael Roth wrote:
>> Define away the annotations so we can still compile.
>>
>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>> ---
>>   qapi/qc.h |   11 +++++++++++
>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>   create mode 100644 qapi/qc.h
>>
>> diff --git a/qapi/qc.h b/qapi/qc.h
>> new file mode 100644
>> index 0000000..3b3a8b9
>> --- /dev/null
>> +++ b/qapi/qc.h
>> @@ -0,0 +1,11 @@
>> +#ifndef QC_H
>> +#define QC_H
>> +
>> +#define qc_declaration
>> +#define _immutable
>> +#define _derived
>> +#define _broken
>> +#define _version(x)
>> +#define _size_is(x)
>> +
>> +#endif
>
> These tags require a different prefix than the reserved '_'.

The rationale is that QIDL is part of the "compiler/library implementation" that 
this namespace is reserved for.

Regards,

Anthony Liguori

>
> Jan
>
Jan Kiszka - June 5, 2012, 11:26 a.m.
On 2012-06-05 13:12, Anthony Liguori wrote:
> On 06/05/2012 06:35 PM, Jan Kiszka wrote:
>> On 2012-06-05 03:00, Michael Roth wrote:
>>> Define away the annotations so we can still compile.
>>>
>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>> ---
>>>   qapi/qc.h |   11 +++++++++++
>>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>>   create mode 100644 qapi/qc.h
>>>
>>> diff --git a/qapi/qc.h b/qapi/qc.h
>>> new file mode 100644
>>> index 0000000..3b3a8b9
>>> --- /dev/null
>>> +++ b/qapi/qc.h
>>> @@ -0,0 +1,11 @@
>>> +#ifndef QC_H
>>> +#define QC_H
>>> +
>>> +#define qc_declaration
>>> +#define _immutable
>>> +#define _derived
>>> +#define _broken
>>> +#define _version(x)
>>> +#define _size_is(x)
>>> +
>>> +#endif
>>
>> These tags require a different prefix than the reserved '_'.
> 
> The rationale is that QIDL is part of the "compiler/library implementation" that 
> this namespace is reserved for.

It's a QEMU-internal thing, and we want to be portable. Not sure if it's
worth to risk collisions in some distant corner.

Jan
Kevin Wolf - June 5, 2012, 11:42 a.m.
Am 05.06.2012 13:26, schrieb Jan Kiszka:
> On 2012-06-05 13:12, Anthony Liguori wrote:
>> On 06/05/2012 06:35 PM, Jan Kiszka wrote:
>>> On 2012-06-05 03:00, Michael Roth wrote:
>>>> Define away the annotations so we can still compile.
>>>>
>>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>>> ---
>>>>   qapi/qc.h |   11 +++++++++++
>>>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>>>   create mode 100644 qapi/qc.h
>>>>
>>>> diff --git a/qapi/qc.h b/qapi/qc.h
>>>> new file mode 100644
>>>> index 0000000..3b3a8b9
>>>> --- /dev/null
>>>> +++ b/qapi/qc.h
>>>> @@ -0,0 +1,11 @@
>>>> +#ifndef QC_H
>>>> +#define QC_H
>>>> +
>>>> +#define qc_declaration
>>>> +#define _immutable
>>>> +#define _derived
>>>> +#define _broken
>>>> +#define _version(x)
>>>> +#define _size_is(x)
>>>> +
>>>> +#endif
>>>
>>> These tags require a different prefix than the reserved '_'.
>>
>> The rationale is that QIDL is part of the "compiler/library implementation" that 
>> this namespace is reserved for.
> 
> It's a QEMU-internal thing, and we want to be portable. Not sure if it's
> worth to risk collisions in some distant corner.

I agree, this isn't really convincing. If QIDL was part of the system
environment, it wouldn't be in the qemu source tree. After all these
rules aren't there just for fun but in order to avoid naming conflicts,
and conflicts between qemu and qemu are much less likely than between
some exotic libc and qemu.

Kevin
Paolo Bonzini - June 5, 2012, 2:08 p.m.
Il 05/06/2012 03:00, Michael Roth ha scritto:
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qapi/qc.h |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
>  create mode 100644 qapi/qc.h
> 
> diff --git a/qapi/qc.h b/qapi/qc.h
> new file mode 100644
> index 0000000..3b3a8b9
> --- /dev/null
> +++ b/qapi/qc.h
> @@ -0,0 +1,11 @@
> +#ifndef QC_H
> +#define QC_H
> +
> +#define qc_declaration
> +#define _immutable
> +#define _derived
> +#define _broken
> +#define _version(x)
> +#define _size_is(x)

Would it be feasible to make the declaration look like the GCC attribute
extension, e.g.

struct RTCState QIDL() {
    int foo QIDL(immutable);
    int bar QIDL(derived);
};

so that you can just use "#define QIDL(...)"?  This is how GCC
developers did their introspection annotations.

Paolo
Michael Roth - June 5, 2012, 9:44 p.m.
On Tue, Jun 05, 2012 at 04:08:55PM +0200, Paolo Bonzini wrote:
> Il 05/06/2012 03:00, Michael Roth ha scritto:
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  qapi/qc.h |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> >  create mode 100644 qapi/qc.h
> > 
> > diff --git a/qapi/qc.h b/qapi/qc.h
> > new file mode 100644
> > index 0000000..3b3a8b9
> > --- /dev/null
> > +++ b/qapi/qc.h
> > @@ -0,0 +1,11 @@
> > +#ifndef QC_H
> > +#define QC_H
> > +
> > +#define qc_declaration
> > +#define _immutable
> > +#define _derived
> > +#define _broken
> > +#define _version(x)
> > +#define _size_is(x)
> 
> Would it be feasible to make the declaration look like the GCC attribute
> extension, e.g.
> 
> struct RTCState QIDL() {
>     int foo QIDL(immutable);
>     int bar QIDL(derived);
> };
> 
> so that you can just use "#define QIDL(...)"?  This is how GCC
> developers did their introspection annotations.

This does seem a lot cleaner to me, also simplifies the lexer and we get
namespacing for free.

> 
> Paolo
>
Anthony Liguori - June 5, 2012, 11:35 p.m.
On 06/06/2012 05:44 AM, Michael Roth wrote:
> On Tue, Jun 05, 2012 at 04:08:55PM +0200, Paolo Bonzini wrote:
>> Il 05/06/2012 03:00, Michael Roth ha scritto:
>>> Signed-off-by: Michael Roth<mdroth@linux.vnet.ibm.com>
>>> ---
>>>   qapi/qc.h |   11 +++++++++++
>>>   1 files changed, 11 insertions(+), 0 deletions(-)
>>>   create mode 100644 qapi/qc.h
>>>
>>> diff --git a/qapi/qc.h b/qapi/qc.h
>>> new file mode 100644
>>> index 0000000..3b3a8b9
>>> --- /dev/null
>>> +++ b/qapi/qc.h
>>> @@ -0,0 +1,11 @@
>>> +#ifndef QC_H
>>> +#define QC_H
>>> +
>>> +#define qc_declaration
>>> +#define _immutable
>>> +#define _derived
>>> +#define _broken
>>> +#define _version(x)
>>> +#define _size_is(x)
>>
>> Would it be feasible to make the declaration look like the GCC attribute
>> extension, e.g.
>>
>> struct RTCState QIDL() {
>>      int foo QIDL(immutable);
>>      int bar QIDL(derived);
>> };
>>
>> so that you can just use "#define QIDL(...)"?  This is how GCC
>> developers did their introspection annotations.
>
> This does seem a lot cleaner to me, also simplifies the lexer and we get
> namespacing for free.

Makes sense to me.

Regards,

Anthony Liguori

>
>>
>> Paolo
>>
>
>

Patch

diff --git a/qapi/qc.h b/qapi/qc.h
new file mode 100644
index 0000000..3b3a8b9
--- /dev/null
+++ b/qapi/qc.h
@@ -0,0 +1,11 @@ 
+#ifndef QC_H
+#define QC_H
+
+#define qc_declaration
+#define _immutable
+#define _derived
+#define _broken
+#define _version(x)
+#define _size_is(x)
+
+#endif