Re: [PATCH 2/4] voicecall: api for set/clear alpha and icon id

> } > > + if (vc->dial_req) { > + v->message = vc->dial_req->message; > + v->icon_id = vc->dial_req->icon_id; > + > + vc->dial_req->message = NULL; > + vc->dial_req->call = v; > + > + newcall->phone_number.type = vc->dial_req->ph.type; > + strncpy(newcall->phone_number.number, vc->dial_req->ph.number, > + 20); Do you really want to do this? I suspect we should override the phone number information only if the CLIP validity is not valid.

> + > + /* > + * TS 102 223 Section 6.4.13: The terminal shall not store > + * in the UICC the call set-up details (called party number > + * and associated parameters) > + */ > + v->untracked = TRUE; > + } > + This looks fine, but there's a very subtle thing going on which breaks one particular case. Some background, voicecall driver is free to notify the call before returning from the atd callback, or vice versa. So e.g. #1 driver->dial(.., cb, data); ofono_voicecall_notify() cb(data); or #2 driver->dial(..., cb, data); cb(data); ofono_voicecall_notify() In the case that oFono generates the Set Up Call Request and assuming driver behavior #1, you end up performing some steps twice. Perhaps we should set a flag in vc->flags to indicate that the call setup is being initiated by the modem, and only trigger this path if this flag is set.

> +void __ofono_voicecall_clear_alpha_and_icon_id(struct ofono_voicecall > *vc) > +{ > + const char *number; > + > + if (vc->dial_req == NULL) > + return; > + > + number = phone_number_to_string(&vc->dial_req->ph); > + > + /* > + * Incase the modem fails to setup the call and if there is no call > + * created, then the emergency mode has to be cleared. > + */ > + if (!strcmp(number, "112")) { > + if (voicecalls_num_active(vc) == 0 && > + voicecalls_num_connecting(vc) == 0) { > + struct ofono_modem *modem = > + __ofono_atom_get_modem(vc->atom); > + > + __ofono_modem_dec_emergency_mode(modem); > + } > + } I'm guessing what you're trying to take care of here is the case that the set up call might have been canceled before the call was notified, right? Wouldn't it be better to do: if (vc->dial_req->call == NULL) ... ? However, in the end I think it is simpler to only initiate the emergency mode when we have the call, and let it be deactivated by the disconnect notification. Then this function becomes way simpler. > + > + g_free(vc->dial_req->message); > + vc->dial_req->message = NULL; > + > + g_free(vc->dial_req); > + vc->dial_req = NULL; > +} > + However, I'm actually questioning the need for this function since the cancellation can only be accomplished by hanging up the call before it is active, in which case the modem will take care of sending the appropriate terminal response.