[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties |
Date: |
Thu, 01 Dec 2011 15:50:58 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20111115 Thunderbird/8.0 |
On 12/01/2011 03:47 PM, Anthony Liguori wrote:
> On 12/01/2011 06:34 AM, Avi Kivity wrote:
>> On 12/01/2011 01:35 PM, Stefan Hajnoczi wrote:
>>>>>
>>>>> +static void qdev_get_link_property(DeviceState *dev, Visitor *v,
>>>>> void *opaque,
>>>>> + const char *name, Error **errp)
>>>>> +{
>>>>> + DeviceState **child = opaque;
>>>>> + gchar *path;
>>>>> +
>>>>> + if (*child) {
>>>>> + path = qdev_get_canonical_path(*child);
>>>>> + visit_type_str(v,&path, name, errp);
>>>>> + g_free(path);
>>>>> + } else {
>>>>> + path = (gchar *)"";
>>>>
>>>> If gchar != char, this is wrong. Also, you're converting a const
>>>> pointer into a non-const pointer, discarding type safety.
>>>
>>> This looked weird to me too but the cast has to do with the fact that
>>> the visitor function works both for input and output visitors. The
>>> output visitor needs to write to gchar** while the input visitor does
>>> not.
>>
>> So you need to pass a non-const pointer to an array of const char, or
>> const gchar **. You don't modify the string in place, you allocate a
>> new string and free the old one.
>>
>>
>>> When this function is called with the correct visitor type we are
>>> guaranteed that path will not be modified.
>>
>> What if it's called with the output visitor? (warning: confusing
>> convention).
>
> The reason there's a single Visitor type that works for both input and
> output visitors is to make it so you can write a single visit function
> that works for input and output. This works very well for most cases
> (in fact, QAPI makes heavy use of it).
>
> That said, I'm starting to feel like there should be a separate input
> and output visitor interface. It would make a number of things (like
> visiting lists) significantly simpler.
Perhaps. But that's independent of the issue here. No matter how many
visitors you have, you never change a gchar in a string in place, you
always allocate a new string (or you can't change the string length).
So the type needs to be const gchar **, not gchar **.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties, Anthony Liguori, 2011/12/01
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties, Gerd Hoffmann, 2011/12/01
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties, Avi Kivity, 2011/12/01
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties, Anthony Liguori, 2011/12/01
Re: [Qemu-devel] [PATCH 07/18] qom: add link properties, Kevin Wolf, 2011/12/02