Re: [Qemu-devel] [PATCH qom-cpu 1/6] cpu: Introduce CPUClass::parse_features() hook

[Andreas Färber]

Am 05.03.2014 23:31, schrieb Eduardo Habkost:
> On Wed, Mar 05, 2014 at 05:57:10PM +0100, Igor Mammedov wrote:
>> On Wed, 05 Mar 2014 17:06:15 +0100
>> Andreas Färber <address@hidden> wrote:
>>
>>> Am 05.03.2014 16:04, schrieb Igor Mammedov:
>>>> On Tue,  4 Mar 2014 03:55:44 +0100
>>>> Andreas Färber <address@hidden> wrote:
>>>>
>>>>> Adapt the X86CPU implementation to suit the generic hook.
>>>>> This involves a cleanup of error handling to cope with NULL errp.
>>>>>
>>>>> Signed-off-by: Andreas Färber <address@hidden>
>>>>> ---
>>>>>  include/qom/cpu.h |  3 +++
>>>>>  target-i386/cpu.c | 36 +++++++++++++++++++++---------------
>>>>>  2 files changed, 24 insertions(+), 15 deletions(-)
>>>>>
>>>>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>>>>> index 64ebfa5..43d253a 100644
>>>>> --- a/include/qom/cpu.h
>>>>> +++ b/include/qom/cpu.h
>>>>> @@ -67,6 +67,8 @@ struct TranslationBlock;
>>>>>   * CPUClass:
>>>>>   * @class_by_name: Callback to map -cpu command line model name to an
>>>>>   * instantiatable CPU type.
>>>>> + * @parse_features: Callback to parse command line arguments.
>>>>> + * The argument may be modified by the callback.
>>>> Could you specify which argument is expected to be modified?
>>>
>>> Like so? "The arguments (%str) may be modified by the callback."
>>>
>>> Alternatively I could drop that line, given that it's not const char *.
>>> Or add a typedef for the callback and document it there using @str syntax.
>> I'd prefer to drop it.
>>
>> BTW: why is 'str' modified by callback?
> 
> Allowing it to be modified allows (for example) strtok() to be used
> (like we do on the i386 code today). So I don't see a reason to forbid
> it.

The second user apart from strtok (which NUL-terminates tokens) is
feat2prop() replacing '_' -> '-' in target-i386.

Dropping:

diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 61134af..3703b68 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -69,7 +69,6 @@ struct TranslationBlock;
  * @class_by_name: Callback to map -cpu command line model name to an
  * instantiatable CPU type.
  * @parse_features: Callback to parse command line arguments.
- * The argument may be modified by the callback.
  * @reset: Callback to reset the #CPUState to its initial state.
  * @reset_dump_flags: #CPUDumpFlags to use for reset logging.
  * @has_work: Callback for checking if there is work to do.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg