Patchwork [04/12] qapi: add "unix" to the set of reserved words

login
register
mail settings
Submitter Paolo Bonzini
Date Sept. 19, 2012, 2:31 p.m.
Message ID <1348065078-5139-5-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/185063/
State New
Headers show

Comments

Paolo Bonzini - Sept. 19, 2012, 2:31 p.m.
It is #defined to 1.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/qapi.py | 4 +++-
 1 file modificato, 3 inserzioni(+). 1 rimozione(-)
Peter Maydell - Sept. 19, 2012, 3:46 p.m.
On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
> It is #defined to 1.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  scripts/qapi.py | 4 +++-
>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 057332e..afc5f32 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -156,7 +156,9 @@ def c_var(name, protect=True):
>      # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
>      # excluding _.*
>      gcc_words = set(['asm', 'typeof'])
> -    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
> +    # namespace pollution:
> +    polluted_words = set(['unix'])
> +    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
>          return "q_" + name
>      return name.replace('-', '_').lstrip("*")
>

I can't help thinking this is fighting a losing battle, and we should just
always prefix everything to avoid clashes.

-- PMM
Paolo Bonzini - Sept. 19, 2012, 3:58 p.m.
Il 19/09/2012 17:46, Peter Maydell ha scritto:
> On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> It is #defined to 1.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  scripts/qapi.py | 4 +++-
>>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 057332e..afc5f32 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -156,7 +156,9 @@ def c_var(name, protect=True):
>>      # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
>>      # excluding _.*
>>      gcc_words = set(['asm', 'typeof'])
>> -    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
>> +    # namespace pollution:
>> +    polluted_words = set(['unix'])
>> +    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
>>          return "q_" + name
>>      return name.replace('-', '_').lstrip("*")
>>
> 
> I can't help thinking this is fighting a losing battle, and we should just
> always prefix everything to avoid clashes.

That would be so ugly that it would be almost useless.  Plus there would
be a huge amount of code to convert.

Paolo
Paolo Bonzini - Sept. 19, 2012, 4:02 p.m.
Il 19/09/2012 17:58, Paolo Bonzini ha scritto:
> Il 19/09/2012 17:46, Peter Maydell ha scritto:
>> On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> It is #defined to 1.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>>  scripts/qapi.py | 4 +++-
>>>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>>>
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index 057332e..afc5f32 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -156,7 +156,9 @@ def c_var(name, protect=True):
>>>      # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
>>>      # excluding _.*
>>>      gcc_words = set(['asm', 'typeof'])
>>> -    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
>>> +    # namespace pollution:
>>> +    polluted_words = set(['unix'])
>>> +    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
>>>          return "q_" + name
>>>      return name.replace('-', '_').lstrip("*")
>>>
>>
>> I can't help thinking this is fighting a losing battle, and we should just
>> always prefix everything to avoid clashes.
> 
> That would be so ugly that it would be almost useless.  Plus there would
> be a huge amount of code to convert.

Also, not really that bad:

$ gcc -dM -x c /dev/null -E|grep define\ [^_]
#define unix 1
#define linux 1

I don't expect other OSes to be significantly worse.  Remember this
breakage is not limited to QAPI-generated code, it would happen in
normal code as well.  I learnt today that a variable named "unix" is not
kosher.

Paolo
Blue Swirl - Sept. 19, 2012, 7:29 p.m.
On Wed, Sep 19, 2012 at 4:02 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 19/09/2012 17:58, Paolo Bonzini ha scritto:
>> Il 19/09/2012 17:46, Peter Maydell ha scritto:
>>> On 19 September 2012 15:31, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>>> It is #defined to 1.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>> ---
>>>>  scripts/qapi.py | 4 +++-
>>>>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
>>>>
>>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>>> index 057332e..afc5f32 100644
>>>> --- a/scripts/qapi.py
>>>> +++ b/scripts/qapi.py
>>>> @@ -156,7 +156,9 @@ def c_var(name, protect=True):
>>>>      # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
>>>>      # excluding _.*
>>>>      gcc_words = set(['asm', 'typeof'])
>>>> -    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
>>>> +    # namespace pollution:
>>>> +    polluted_words = set(['unix'])
>>>> +    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
>>>>          return "q_" + name
>>>>      return name.replace('-', '_').lstrip("*")
>>>>
>>>
>>> I can't help thinking this is fighting a losing battle, and we should just
>>> always prefix everything to avoid clashes.
>>
>> That would be so ugly that it would be almost useless.  Plus there would
>> be a huge amount of code to convert.
>
> Also, not really that bad:
>
> $ gcc -dM -x c /dev/null -E|grep define\ [^_]
> #define unix 1
> #define linux 1
>
> I don't expect other OSes to be significantly worse.  Remember this
> breakage is not limited to QAPI-generated code, it would happen in
> normal code as well.  I learnt today that a variable named "unix" is not
> kosher.

I got only this from OpenBSD:
#define sparc 1

Mingw has these:
#define WIN32 1
#define WINNT 1
#define i386 1

I'd suppose the full list from GCC is this:
$ grep -hr 'builtin_define_std' gcc/config |sed -n
's/.*"\([^"]*\)".*/\1/p'|sort -u
AVR
BFIN
CRIS
GNU_CRIS
GO32
LANGUAGE_ASSEMBLY
LANGUAGE_C
LANGUAGE_C_PLUS_PLUS
LANGUAGE_OBJECTIVE_C
MACH
MIPSEB
MIPSEL
MOXIE
MSDOS
PPC
R3000
R4000
REVARGV
SYSTYPE_BSD
SYSTYPE_SVR4
VMS
WIN32
WIN64
WINNT
bfin
cris
fr30
h8300
host_mips
hp800
hp9000
hp9k8
hpux
i386
linux
mc68000
mc68010
mc68020
mc68030
mc68040
mc68060
mc68332
mcpu32
mep
moxie
pdp11
powerpc
sgi
sparc
spectrum
sun
tpf
unix
vms
xstormy16

>
> Paolo
>
Luiz Capitulino - Sept. 20, 2012, 2:08 p.m.
On Wed, 19 Sep 2012 16:31:07 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> It is #defined to 1.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Cherry-picked into qmp branch, thanks.

> ---
>  scripts/qapi.py | 4 +++-
>  1 file modificato, 3 inserzioni(+). 1 rimozione(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 057332e..afc5f32 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -156,7 +156,9 @@ def c_var(name, protect=True):
>      # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
>      # excluding _.*
>      gcc_words = set(['asm', 'typeof'])
> -    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
> +    # namespace pollution:
> +    polluted_words = set(['unix'])
> +    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
>          return "q_" + name
>      return name.replace('-', '_').lstrip("*")
>

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 057332e..afc5f32 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -156,7 +156,9 @@  def c_var(name, protect=True):
     # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
     # excluding _.*
     gcc_words = set(['asm', 'typeof'])
-    if protect and (name in c89_words | c99_words | c11_words | gcc_words):
+    # namespace pollution:
+    polluted_words = set(['unix'])
+    if protect and (name in c89_words | c99_words | c11_words | gcc_words | polluted_words):
         return "q_" + name
     return name.replace('-', '_').lstrip("*")