Re: [PATCH -v3 5/5] modem: add support to restore state when resetting the modem

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

Hi Gustavo,

> @@ -488,16 +465,22 @@ static void online_cb(const struct ofono_error *error, void *data)
>  	DBusMessage *reply;
>  
>  	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
> -			modem->modem_state == MODEM_STATE_OFFLINE)
> +			modem->modem_state == MODEM_STATE_OFFLINE) {
> +		modem_change_state(modem, MODEM_STATE_ONLINE);

This actually breaks the (yet unwritten) rule to try to return from the
method call before signaling the property changed.

Can you leave this be and add something like:

if (modem->pending == NULL)
	goto out;

or something prior to this if condition?

> +
> +		if (!modem->pending)
> +			return;
> +

Then you can omit this

>  		reply = dbus_message_new_method_return(modem->pending);
> -	else
> +	} else {
> +
> +		if (!modem->pending)
> +			return;
> +

And this

>  		reply = __ofono_error_failed(modem->pending);
> +	}
>  
>  	__ofono_dbus_pending_reply(&modem->pending, reply);

And just add a label here...

> -
> -	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
> -			modem->modem_state == MODEM_STATE_OFFLINE)
> -		modem_change_state(modem, MODEM_STATE_ONLINE);
>  }
>  
>  static void offline_cb(const struct ofono_error *error, void *data)

Regards,
-Denis