[PATCH 0/3] CPU frequency scaling addon

[Holger Macht]

On Mon 14. Aug - 21:20:00, Richard Hughes wrote:
> On Fri, 2006-08-11 at 19:44 +0200, Holger Macht wrote:
> > The following patches add CPU frequency capabilities to HAL via an addon.
> > This was already discussed in another thread [1].
> 
> Also, another oddity I found:
> 
> dbus-send --system --print-reply
> --dest=org.freedesktop.Hal /org/freedesktop/Hal/devices/computer
> org.freedesktop.Hal.Device.SystemPowerManagement.GetCPUFreqAvailableGovernors
> 
> method return sender=:1.50 -> dest=:1.61
>    string "userspace"
>    string "performance"
> 
> which uses dbus_send_reply_strlist (a method that just appends string
> return arguments to the message), whilst:
> 
> dbus-send --system --print-reply
> --dest=org.freedesktop.Hal /org/freedesktop/Hal/Manager
> org.freedesktop.Hal.Manager.GetAllDevices
> method return sender=:1.50 -> dest=:1.62
>    array [
>       string "/org/freedesktop/Hal/devices/acpi_lcd"
>       string "/org/freedesktop/Hal/devices/acpi_PWRB"
>       string "/org/freedesktop/Hal/devices/computer"
>    ]
> 
> which uses:
> 
> dbus_message_iter_open_container (&iter, 
> 	  DBUS_TYPE_ARRAY,
> 	  DBUS_TYPE_STRING_AS_STRING,
> 	  &iter_array);
> hal_device_store_foreach (hald_get_gdl (),
> 	  foreach_device_get_udi,
> 	  &iter_array);
> dbus_message_iter_close_container (&iter, &iter_array);
> 
> I'm guessing the addon needs to return an array of strings, rather than
> a variable message length list of strings. I know changing the message
> format depending on the number of returned values breaks the dbus-glib
> bindings for me, and I'm guessing the python bindings the same.

I don't really understand that. Can you please elaborate a bit why this
should break any bindings?

> 
> Patch (attached) on top of your patch attached fixes the issue, making
> the return type 'as' which matches most of the other HAL methods.

Ok, I will integrate this patch into the next patch I'm sending as soon as
I understand that it is needed. At the moment, I don't have a concrete
preference for any of the two methods ;-)

> 
> Richard
> 

Thanks,
	Holger


> --- addon-cpufreq.c.old	2006-08-14 19:31:52.000000000 +0100
> +++ addon-cpufreq.c	2006-08-14 21:09:39.000000000 +0100
> @@ -905,6 +905,8 @@ static gboolean dbus_send_reply_strlist(
>  					gchar **list)
>  {
>  	DBusMessage *reply;
> +	DBusMessageIter iter;
> +	DBusMessageIter iter_array;
>  	int	    i;
>  
>  	if ((reply = dbus_message_new_method_return(message)) == NULL) {
> @@ -912,8 +914,17 @@ static gboolean dbus_send_reply_strlist(
>  		return FALSE;
>  	}
>  
> -	for (i = 0; list[i] != NULL; i++)
> -		dbus_message_append_args(reply, DBUS_TYPE_STRING, &list[i], DBUS_TYPE_INVALID);			
> +	dbus_message_iter_init_append (reply, &iter);
> +	dbus_message_iter_open_container (&iter, 
> +					  DBUS_TYPE_ARRAY,
> +					  DBUS_TYPE_STRING_AS_STRING,
> +					  &iter_array);
> +
> +	for (i = 0; list[i] != NULL; i++) {
> +		dbus_message_iter_append_basic (&iter_array, DBUS_TYPE_STRING, &list[i]);
> +	}
> +
> +	dbus_message_iter_close_container (&iter, &iter_array);
>  
>  	if (!dbus_connection_send(connection, reply, NULL)) {
>  		ERROR("Could not sent reply");
> @@ -1114,7 +1125,7 @@ gboolean dbus_init(void)
>  		"      <arg name=\"return_code\" direction=\"out\" type=\"b\"/>"
>  		"    </method>"
>  		"    <method name=\"GetCPUFreqAvailableGovernors\">"
> -		"      <arg name=\"return_code\" direction=\"out\" type=\"strlist\"/>"
> +		"      <arg name=\"return_code\" direction=\"out\" type=\"as\"/>"
>  		"    </method>",
>  		&dbus_error)) {
>  

> _______________________________________________
> hal mailing list
> hal at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/hal