Patchwork [01/10] qom: Make object_resolve_path_component() path argument const

login
register
mail settings
Submitter Andreas Färber
Date Jan. 13, 2013, 11:54 p.m.
Message ID <1358121304-21345-2-git-send-email-afaerber@suse.de>
Download mbox | patch
Permalink /patch/211694/
State New
Headers show

Comments

Andreas Färber - Jan. 13, 2013, 11:54 p.m.
This allows to navigate partial well-known paths from an object.

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(-)
Markus Armbruster - Jan. 14, 2013, 12:19 p.m.
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?

> 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?
Andreas Färber - Jan. 14, 2013, 4:52 p.m.
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
Markus Armbruster - Jan. 14, 2013, 5:27 p.m.
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.
Markus Armbruster - Jan. 14, 2013, 7:01 p.m.
[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
Blue Swirl - Jan. 17, 2013, 8:26 p.m.
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
>
Markus Armbruster - Jan. 18, 2013, 3:36 p.m.
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!!!

[...]
Blue Swirl - Jan. 19, 2013, 9:13 a.m.
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

>
> [...]

Patch

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) {