Re: [PATCH] core: improving handling of rfkill

[Dan Williams <dcbw redhat com>]

On Tue, 2011-09-20 at 16:36 +0800, Gary Ching-Pang Lin wrote:
> This commit improves the handling of rfkill.
> 
> - The original two passes check gathers the states of platform
>   and non-platform switches in two separate loops. Now we gather
>   the both states in one loop and determine the final states later.
> 
> - A new rule is used to determine the states of switches.
> 
>   if (platform_state == UNBLOCKED)
>           choose non_platform_state;
>   else
>           choose platform_state;
> 
>   The state is UNBLOCKED if and only if both the platform and
>   non-platform switches are unblocked, so the ambiguous state in
>   bgo#655773 will not happen.
>   See: https://bugzilla.gnome.org/show_bug.cgi?id=655773

Pushed, thanks.

Dan

> ---
>  src/nm-udev-manager.c |   73 ++++++++++++++++---------------------------------
>  1 files changed, 24 insertions(+), 49 deletions(-)
> 
> diff --git a/src/nm-udev-manager.c b/src/nm-udev-manager.c
> index 72501c2..73c1f44 100644
> --- a/src/nm-udev-manager.c
> +++ b/src/nm-udev-manager.c
> @@ -189,84 +189,59 @@ sysfs_state_to_nm_state (gint sysfs_state)
>  	return RFKILL_UNBLOCKED;
>  }
>  
> +static RfKillState
> +aggregate_states (RfKillState platform, RfKillState non_platform)
> +{
> +	if (platform == RFKILL_UNBLOCKED)
> +		return non_platform;
> +	else
> +		return platform;
> +}
> +
>  static void
>  recheck_killswitches (NMUdevManager *self)
>  {
>  	NMUdevManagerPrivate *priv = NM_UDEV_MANAGER_GET_PRIVATE (self);
>  	GSList *iter;
>  	RfKillState poll_states[RFKILL_TYPE_MAX];
> +	RfKillState platform_states[RFKILL_TYPE_MAX];
>  	gboolean platform_checked[RFKILL_TYPE_MAX];
>  	int i;
>  
>  	/* Default state is unblocked */
>  	for (i = 0; i < RFKILL_TYPE_MAX; i++) {
>  		poll_states[i] = RFKILL_UNBLOCKED;
> +		platform_states[i] = RFKILL_UNBLOCKED;
>  		platform_checked[i] = FALSE;
>  	}
>  
> -	/* Perform two passes here; the first pass is for non-platform switches,
> -	 * which typically if hardkilled cannot be changed except by a physical
> -	 * hardware switch.  The second pass checks platform killswitches, which
> -	 * take precedence over device killswitches, because typically platform
> -	 * killswitches control device killswitches.  That is, a hardblocked device
> -	 * switch can often be unblocked by a platform switch.  Thus if we have
> -	 * a hardblocked device switch and a softblocked platform switch, the
> -	 * combined state should be softblocked since the platform switch can be
> -	 * unblocked to change the device switch.
> -	 */
> -
> -	/* Device switches first */
> +	/* Gather the states of platform and non-platform switches */
>  	for (iter = priv->killswitches; iter; iter = g_slist_next (iter)) {
>  		Killswitch *ks = iter->data;
>  		GUdevDevice *device;
>  		RfKillState dev_state;
>  		int sysfs_state;
>  
> -		if (ks->platform == FALSE) {
> -			device = g_udev_client_query_by_subsystem_and_name (priv->client, "rfkill", ks->name);
> -			if (device) {
> -				sysfs_state = g_udev_device_get_property_as_int (device, "RFKILL_STATE");
> -				dev_state = sysfs_state_to_nm_state (sysfs_state);
> +		device = g_udev_client_query_by_subsystem_and_name (priv->client, "rfkill", ks->name);
> +		if (device) {
> +			sysfs_state = g_udev_device_get_property_as_int (device, "RFKILL_STATE");
> +			dev_state = sysfs_state_to_nm_state (sysfs_state);
> +			if (ks->platform == FALSE) {
>  				if (dev_state > poll_states[ks->rtype])
>  					poll_states[ks->rtype] = dev_state;
> -				g_object_unref (device);
> -			}
> -		}
> -	}
> -
> -	/* Platform switches next; their state overwrites device state */
> -	for (iter = priv->killswitches; iter; iter = g_slist_next (iter)) {
> -		Killswitch *ks = iter->data;
> -		GUdevDevice *device;
> -		RfKillState dev_state;
> -		int sysfs_state;
> -
> -		if (ks->platform == TRUE) {
> -			device = g_udev_client_query_by_subsystem_and_name (priv->client, "rfkill", ks->name);
> -			if (device) {
> -				sysfs_state = g_udev_device_get_property_as_int (device, "RFKILL_STATE");
> -				dev_state = sysfs_state_to_nm_state (sysfs_state);
> -
> -				if (platform_checked[ks->rtype] == FALSE) {
> -					/* Overwrite device state with platform state for first
> -					 * platform switch found.
> -					 */
> -					poll_states[ks->rtype] = dev_state;
> -					platform_checked[ks->rtype] = TRUE;
> -				} else {
> -					/* If there are multiple platform switches of the same type,
> -					 * take the "worst" state for all of that type.
> -					 */
> -					if (dev_state > poll_states[ks->rtype])
> -						poll_states[ks->rtype] = dev_state;
> -				}
> -				g_object_unref (device);
> +			} else {
> +				platform_checked[ks->rtype] = TRUE;
> +				if (dev_state > platform_states[ks->rtype])
> +					platform_states[ks->rtype] = dev_state;
>  			}
> +			g_object_unref (device);
>  		}
>  	}
>  
>  	/* Log and emit change signal for final rfkill states */
>  	for (i = 0; i < RFKILL_TYPE_MAX; i++) {
> +		if (platform_checked[i] == TRUE)
> +			poll_states[i] = aggregate_states (platform_states[i], poll_states[i]);
>  		if (poll_states[i] != priv->rfkill_states[i]) {
>  			nm_log_dbg (LOGD_RFKILL, "%s rfkill state now '%s'",
>  			            rfkill_type_to_desc (i),