Hi Pekka,

On 01/14/2011 08:40 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Allow use of GPRS atom both in online and offline (post_sim) states.
> 
> The GPRS now considers the removal of the netreg atom as implicit detach
> and registration change to NOT_REGISTERED.
> ---
>  src/gprs.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gprs.c b/src/gprs.c
> index 0e86bdf..35cc475 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -1370,10 +1370,27 @@ static void gprs_attach_callback(const struct ofono_error *error, void *data)
>  	}
>  }
>  
> +static void gprs_netreg_removed(struct ofono_gprs *gprs)
> +{
> +	gprs->netreg = NULL;
> +
> +	gprs->flags &= ~(GPRS_FLAG_RECHECK | GPRS_FLAG_ATTACHING);
> +	gprs->status_watch = 0;
> +	gprs->netreg_status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
> +	gprs->driver_attached = FALSE;
> +
> +	gprs_attached_update(gprs);
> +}
> +
>  static void gprs_netreg_update(struct ofono_gprs *gprs)
>  {
>  	ofono_bool_t attach;
>  
> +	if (gprs->netreg == NULL) {
> +		gprs_netreg_removed(gprs);
> +		return;
> +	}
> +

I'm having trouble visualizing why this chunk is necessary.  We reset
netreg to NULL only when shutting down the atom or when netreg atom goes
away.  So this condition should never be triggered.

>  	attach = gprs->netreg_status == NETWORK_REGISTRATION_STATUS_REGISTERED;
>  
>  	attach = attach || (gprs->roaming_allowed &&
> @@ -2250,8 +2267,7 @@ static void netreg_watch(struct ofono_atom *atom,
>  	struct ofono_gprs *gprs = data;
>  
>  	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
> -		gprs->status_watch = 0;
> -		gprs->netreg = NULL;
> +		gprs_netreg_removed(gprs);
>  		return;
>  	}
>  

Otherwise the rest seems fine to me.

Regards,
-Denis