Re: [PATCH v3 1/1] qga: add command 'guest-get-cpustats'

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> On Wed, Jul 6, 2022 at 1:56 PM zhenwei pi <pizhenwei@bytedance.com> wrote:
>>
>> A vCPU thread always reaches 100% utilization when:
>> - guest uses idle=poll
>> - disable HLT vm-exit
>> - enable MWAIT
>>
>> Add new guest agent command 'guest-get-cpustats' to get guest CPU
>> statistics, we can know the guest workload and how busy the CPU is.
>>
>> To avoid compiling error like:
>> qga/qga-qapi-types.h:948:28: error: expected member name or ';'
>>  after declaration specifiers
>>         GuestLinuxCpuStats linux;
>>         ~~~~~~~~~~~~~~~~~~ ^
>> <built-in>:336:15: note: expanded from here

What actually happens here: the compiler predefines macro linux like

    #define linux 1

>> Also add 'linux' into polluted_words.

Does polluted_words cover all of the predefined macros then?
Checking...

    $ gcc -E -dD - </dev/null 2>&1 | grep 'define [^_]'
    #define linux 1
    #define unix 1

Yes.  We don't care about the ones starting with '_'.

> That looks good to me, although you may want to split the scripts/qapi change.
>
> Markus, what do you think?

I'd very much prefer a separate patch.  Suggested commit message:

    qapi: Avoid generating C identifier 'linux'

    'linux' is not usable as identifier, because C compilers targeting
    Linux predefine it as a macro expanding to 1.  Add it to
    @polluted_words.  'unix' is already there.

>> Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

[...]

>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
>> index 489273574a..737b059e62 100644
>> --- a/scripts/qapi/common.py
>> +++ b/scripts/qapi/common.py
>> @@ -114,7 +114,7 @@ def c_name(name: str, protect: bool = True) -> str:
>>                       'and', 'and_eq', 'bitand', 'bitor', 'compl', 'not',
>>                       'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
>>      # namespace pollution:
>> -    polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386'])
>> +    polluted_words = set(['unix', 'errno', 'mips', 'sparc', 'i386', 
>> 'linux'])
>>      name = re.sub(r'[^A-Za-z0-9_]', '_', name)
>>      if protect and (name in (c89_words | c99_words | c11_words | gcc_words
>>                               | cpp_words | polluted_words)
>> --
>> 2.20.1
>>