Re: [Qemu-trivial] [Qemu-devel] [PATCH 01/10] qom: Make object_resolve_path_component() path argument const

Re: [Qemu-trivial] [Qemu-devel] [PATCH 01/10] qom: Make object_resolve_path_component() path argument const

[Andreas Färber]

Am 14.01.2013 13:19, schrieb Markus Armbruster:
> Andreas Färber <afaerber@suse.de> writes:
> 
>> This allows to navigate partial well-known paths from an object.
> 
> Why does making the argument const allow such navigation?

Without const, object_resolve_path_component(foo, "bar") results in a
compile error (09/10 accesses "ide[1]" etc. to avoid exposing the full
MacIOState).

Apparently this function was so far only used on dynamically generated
(non-const) arrays:

qom/container.c:        child = object_resolve_path_component(obj,
parts[i]);
qom/object.c:    child = object_resolve_path_component(parent,
parts[index]);

>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Cc: Anthony Liguori <anthony@codemonkey.ws>
>> ---
>>  include/qom/object.h |    2 +-
>>  qom/object.c         |    2 +-
>>  2 Dateien geändert, 2 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>>
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index d43b289..1ef2f0e 100644
>> --- a/include/qom/object.h
>> +++ b/include/qom/object.h
>> @@ -900,7 +900,7 @@ Object *object_resolve_path_type(const char *path, const char *typename,
>>   *
>>   * Returns: The resolved object or NULL on path lookup failure.
>>   */
>> -Object *object_resolve_path_component(Object *parent, gchar *part);
>> +Object *object_resolve_path_component(Object *parent, const gchar *part);
>>  /**
>>   * object_property_add_child:
>> diff --git a/qom/object.c b/qom/object.c
>> index 351b88c..03e6f24 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -1017,7 +1017,7 @@ gchar *object_get_canonical_path(Object *obj)
>>      return newpath;
>>  }
>>  
>> -Object *object_resolve_path_component(Object *parent, gchar *part)
>> +Object *object_resolve_path_component(Object *parent, const gchar *part)
>>  {
>>      ObjectProperty *prop = object_property_find(parent, part, NULL);
>>      if (prop == NULL) {
> 
> Unrelated: do we really want to go along with glib's basic type typedef
> silliness?

Elsewhere I have adopted the exact GLib signature since typedefs can be
changed at any time. In particular the GCompareFunc using gconstpointer,
gint, etc. Not saying I find their GLib usage useful.

I admit I didn't review object_property_find(), which uses const char*.
So I wouldn't mind sending a v2 if non-g is preferred. Or Anthony or
Stefan could just edit the patch in my name before applying. :)

Andreas

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

Re: [Qemu-trivial] [Qemu-devel] [PATCH 01/10] qom: Make object_resolve_path_component() path argument const

[Markus Armbruster]

Andreas Färber <afaerber@suse.de> writes:

> Am 14.01.2013 13:19, schrieb Markus Armbruster:
>> Andreas Färber <afaerber@suse.de> writes:
>> 
>>> This allows to navigate partial well-known paths from an object.
>> 
>> Why does making the argument const allow such navigation?
>
> Without const, object_resolve_path_component(foo, "bar") results in a
> compile error (09/10 accesses "ide[1]" etc. to avoid exposing the full
> MacIOState).
>
> Apparently this function was so far only used on dynamically generated
> (non-const) arrays:
>
> qom/container.c:        child = object_resolve_path_component(obj,
> parts[i]);
> qom/object.c:    child = object_resolve_path_component(parent,
> parts[index]);

Makes sense, but isn't immediately obvious from your commit message.
What about:

    qom: Make object_resolve_path_component() path argument const

    Navigating well-known paths from an object like

        object_resolve_path_component(foo, "bar")

    is perfectly legitimate, but doesn't compile, because the second
    parameter lacks const.  Fix that.

[Qemu-trivial] Go along with glib's basic type typedef silliness? (was: [Qemu-devel] [PATCH 01/10] qom: Make object_resolve_path_component() path argument const)

[Markus Armbruster]

[Thread hijack, dropping cc: qemu-trivial, qemu-ppc]

Andreas Färber <afaerber@suse.de> writes:

> Am 14.01.2013 13:19, schrieb Markus Armbruster:
>> Andreas Färber <afaerber@suse.de> writes:
[...]
>>> diff --git a/qom/object.c b/qom/object.c
>>> index 351b88c..03e6f24 100644
>>> --- a/qom/object.c
>>> +++ b/qom/object.c
>>> @@ -1017,7 +1017,7 @@ gchar *object_get_canonical_path(Object *obj)
>>>      return newpath;
>>>  }
>>>  
>>> -Object *object_resolve_path_component(Object *parent, gchar *part)
>>> +Object *object_resolve_path_component(Object *parent, const gchar *part)
>>>  {
>>>      ObjectProperty *prop = object_property_find(parent, part, NULL);
>>>      if (prop == NULL) {
>> 
>> Unrelated: do we really want to go along with glib's basic type typedef
>> silliness?
>
> Elsewhere I have adopted the exact GLib signature since typedefs can be
> changed at any time. In particular the GCompareFunc using gconstpointer,
> gint, etc. Not saying I find their GLib usage useful.

No, these typedefs cannot be changed.

Firstly, their exact definitions are documented[*], therefore can be
relied on.

Secondly, mountains of code rely on the exact definitions, and would
break left and right if they were changed.

They're a textbook example of a perfectly useless pseudo-abstraction.

[...]


[*] http://developer.gnome.org/glib/stable/glib-Basic-Types.html

Re: [Qemu-trivial] [Qemu-devel] Go along with glib's basic type typedef silliness? (was: [PATCH 01/10] qom: Make object_resolve_path_component() path argument const)

[Blue Swirl]

On Mon, Jan 14, 2013 at 7:01 PM, Markus Armbruster <armbru@redhat.com> wrote:
> [Thread hijack, dropping cc: qemu-trivial, qemu-ppc]
>
> Andreas Färber <afaerber@suse.de> writes:
>
>> Am 14.01.2013 13:19, schrieb Markus Armbruster:
>>> Andreas Färber <afaerber@suse.de> writes:
> [...]
>>>> diff --git a/qom/object.c b/qom/object.c
>>>> index 351b88c..03e6f24 100644
>>>> --- a/qom/object.c
>>>> +++ b/qom/object.c
>>>> @@ -1017,7 +1017,7 @@ gchar *object_get_canonical_path(Object *obj)
>>>>      return newpath;
>>>>  }
>>>>
>>>> -Object *object_resolve_path_component(Object *parent, gchar *part)
>>>> +Object *object_resolve_path_component(Object *parent, const gchar *part)
>>>>  {
>>>>      ObjectProperty *prop = object_property_find(parent, part, NULL);
>>>>      if (prop == NULL) {
>>>
>>> Unrelated: do we really want to go along with glib's basic type typedef
>>> silliness?
>>
>> Elsewhere I have adopted the exact GLib signature since typedefs can be
>> changed at any time. In particular the GCompareFunc using gconstpointer,
>> gint, etc. Not saying I find their GLib usage useful.
>
> No, these typedefs cannot be changed.
>
> Firstly, their exact definitions are documented[*], therefore can be
> relied on.
>
> Secondly, mountains of code rely on the exact definitions, and would
> break left and right if they were changed.
>
> They're a textbook example of a perfectly useless pseudo-abstraction.

CONST, WORD, PWORD, DWORD, FLOAT etc. in a certain platform...

>
> [...]
>
>
> [*] http://developer.gnome.org/glib/stable/glib-Basic-Types.html
>

Re: [Qemu-trivial] [Qemu-devel] Go along with glib's basic type typedef silliness?

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> On Mon, Jan 14, 2013 at 7:01 PM, Markus Armbruster <armbru@redhat.com> wrote:
>> [Thread hijack, dropping cc: qemu-trivial, qemu-ppc]
>>
>> Andreas Färber <afaerber@suse.de> writes:
>>
>>> Am 14.01.2013 13:19, schrieb Markus Armbruster:
>>>> Andreas Färber <afaerber@suse.de> writes:
>> [...]
>>>>> diff --git a/qom/object.c b/qom/object.c
>>>>> index 351b88c..03e6f24 100644
>>>>> --- a/qom/object.c
>>>>> +++ b/qom/object.c
>>>>> @@ -1017,7 +1017,7 @@ gchar *object_get_canonical_path(Object *obj)
>>>>>      return newpath;
>>>>>  }
>>>>>
>>>>> -Object *object_resolve_path_component(Object *parent, gchar *part)
>>>>> +Object *object_resolve_path_component(Object *parent, const gchar *part)
>>>>>  {
>>>>>      ObjectProperty *prop = object_property_find(parent, part, NULL);
>>>>>      if (prop == NULL) {
>>>>
>>>> Unrelated: do we really want to go along with glib's basic type typedef
>>>> silliness?
>>>
>>> Elsewhere I have adopted the exact GLib signature since typedefs can be
>>> changed at any time. In particular the GCompareFunc using gconstpointer,
>>> gint, etc. Not saying I find their GLib usage useful.
>>
>> No, these typedefs cannot be changed.
>>
>> Firstly, their exact definitions are documented[*], therefore can be
>> relied on.
>>
>> Secondly, mountains of code rely on the exact definitions, and would
>> break left and right if they were changed.
>>
>> They're a textbook example of a perfectly useless pseudo-abstraction.
>
> CONST, WORD, PWORD, DWORD, FLOAT etc. in a certain platform...

MY EYES, MY EYES, WHAT DID YOU DO TO MY EYES!!!

[...]

Re: [Qemu-trivial] [Qemu-devel] Go along with glib's basic type typedef silliness?

[Blue Swirl]

On Fri, Jan 18, 2013 at 3:36 PM, Markus Armbruster <armbru@redhat.com> wrote:
> Blue Swirl <blauwirbel@gmail.com> writes:
>
>> On Mon, Jan 14, 2013 at 7:01 PM, Markus Armbruster <armbru@redhat.com> wrote:
>>> [Thread hijack, dropping cc: qemu-trivial, qemu-ppc]
>>>
>>> Andreas Färber <afaerber@suse.de> writes:
>>>
>>>> Am 14.01.2013 13:19, schrieb Markus Armbruster:
>>>>> Andreas Färber <afaerber@suse.de> writes:
>>> [...]
>>>>>> diff --git a/qom/object.c b/qom/object.c
>>>>>> index 351b88c..03e6f24 100644
>>>>>> --- a/qom/object.c
>>>>>> +++ b/qom/object.c
>>>>>> @@ -1017,7 +1017,7 @@ gchar *object_get_canonical_path(Object *obj)
>>>>>>      return newpath;
>>>>>>  }
>>>>>>
>>>>>> -Object *object_resolve_path_component(Object *parent, gchar *part)
>>>>>> +Object *object_resolve_path_component(Object *parent, const gchar *part)
>>>>>>  {
>>>>>>      ObjectProperty *prop = object_property_find(parent, part, NULL);
>>>>>>      if (prop == NULL) {
>>>>>
>>>>> Unrelated: do we really want to go along with glib's basic type typedef
>>>>> silliness?
>>>>
>>>> Elsewhere I have adopted the exact GLib signature since typedefs can be
>>>> changed at any time. In particular the GCompareFunc using gconstpointer,
>>>> gint, etc. Not saying I find their GLib usage useful.
>>>
>>> No, these typedefs cannot be changed.
>>>
>>> Firstly, their exact definitions are documented[*], therefore can be
>>> relied on.
>>>
>>> Secondly, mountains of code rely on the exact definitions, and would
>>> break left and right if they were changed.
>>>
>>> They're a textbook example of a perfectly useless pseudo-abstraction.
>>
>> CONST, WORD, PWORD, DWORD, FLOAT etc. in a certain platform...
>
> MY EYES, MY EYES, WHAT DID YOU DO TO MY EYES!!!

Leishmaniasis Capslockititis Fenestrae? :-D

>
> [...]