Re: [PATCH 1/2] voicecall: manage multiparty list in AT+CHLD=3

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Frédéric,

On 06/29/2011 02:38 PM, Frédéric Dalleau wrote:
> ---
>  src/voicecall.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 64 insertions(+), 1 deletions(-)
> 
> diff --git a/src/voicecall.c b/src/voicecall.c
> index 9620838..8e4ab58 100644
> --- a/src/voicecall.c
> +++ b/src/voicecall.c
> @@ -94,6 +94,12 @@ struct voicecall {
>  	ofono_bool_t remote_multiparty;
>  };
>  
> +struct mpty_cb_info {
> +	struct ofono_voicecall *vc;
> +	struct ofono_emulator *em;
> +	int id;
> +};
> +
>  struct dial_request {
>  	struct ofono_voicecall *vc;
>  	char *message;
> @@ -2710,6 +2716,48 @@ static void emulator_generic_cb(const struct ofono_error *error, void *data)
>  	ofono_emulator_send_final(em, error);
>  }
>  
> +static void emulator_mpty_join_cb(const struct ofono_error *error, void *data)
> +{
> +	struct mpty_cb_info *mp = data;
> +	GSList *old;
> +
> +	ofono_emulator_send_final(mp->em, error);
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		g_free(mp);
> +		return;
> +	}
> +
> +	/*
> +	 * We just created a multiparty call, gather all held
> +	 * active calls and add them to the multiparty list
> +	 */
> +	old = mp->vc->multiparty_list;
> +	mp->vc->multiparty_list = 0;
> +
> +	mp->vc->multiparty_list = g_slist_concat(mp->vc->multiparty_list,
> +						voicecalls_held_list(mp->vc));
> +
> +	mp->vc->multiparty_list = g_slist_concat(mp->vc->multiparty_list,
> +						voicecalls_active_list(mp->vc));
> +
> +	mp->vc->multiparty_list = g_slist_sort(mp->vc->multiparty_list,
> +						call_compare);
> +
> +	if (g_slist_length(mp->vc->multiparty_list) < 2) {
> +		ofono_error("Created multiparty call, but size is less than 2"
> +				" panic!");
> +
> +		return;
> +	}
> +
> +	voicecalls_multiparty_changed(old, mp->vc->multiparty_list);
> +	g_slist_free(old);
> +	g_free(mp);
> +}
> +
> +}
> +

Something is wrong with your git-foo here:
src/voicecall.c:2759: error: expected identifier or ‘(’ before ‘}’ token

>  static void emulator_ata_cb(struct ofono_emulator *em,
>  			struct ofono_emulator_request *req, void *userdata)
>  {
> @@ -2900,12 +2948,27 @@ static void emulator_chld_cb(struct ofono_emulator *em,
>  					emulator_generic_cb, em);
>  			return;
>  		case 3:
> +		{
> +			struct mpty_cb_info *mp;
> +
>  			if (vc->driver->create_multiparty == NULL)
>  				goto fail;
>  
> +			if (!voicecalls_have_held(vc)
> +					|| !voicecalls_have_active(vc))
> +				goto fail;
> +
> +			mp = g_try_malloc0(sizeof(struct mpty_cb_info));
> +			if (!mp)
> +				goto fail;
> +
> +			mp->vc = vc;
> +			mp->em = em;
> +
>  			vc->driver->create_multiparty(vc,
> -					emulator_generic_cb, em);
> +					emulator_mpty_join_cb, mp);

So unfortunately you cannot do this, the current oFono driver framework
does not have a mechanism to guarantee the destruction of userdata you
pass to the driver method.  There was never a reason to have it...
Since atoms can and are forcefully removed at any time, you would
potentially be leaking memory here.

Can you re-use the existing pending_em member of ofono_voicecall for
this purpose?

>  			return;
> +		}
>  		case 4:
>  			if (vc->driver->transfer == NULL)
>  				goto fail;

Regards,
-Denis