Hi Kuba,

On 05/18/2015 05:38 AM, Kuba Pawlak wrote:
> If there is more then one active or held call, we are in mpty calls.
> We won't get indicator update if any of them is released by CHLD=1x.
> So we have to poll it.
> ---
>   drivers/hfpmodem/voicecall.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
>

I took another look at this one, a few minor issues:

> diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
> index afeb35f..5d044a9 100644
> --- a/drivers/hfpmodem/voicecall.c
> +++ b/drivers/hfpmodem/voicecall.c
> @@ -1134,6 +1134,10 @@ static void hfp_clcc_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   	struct ofono_voicecall *vc = user_data;
>   	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
>   	unsigned int mpty_ids;
> +	GSList *n;
> +	struct ofono_call *nc;
> +	unsigned int num_active = 0;
> +	unsigned int num_held = 0;
>
>   	if (!ok)
>   		return;
> @@ -1142,6 +1146,22 @@ static void hfp_clcc_cb(gboolean ok, GAtResult *result, gpointer user_data)
>
>   	g_slist_foreach(vd->calls, voicecall_notify, vc);
>   	ofono_voicecall_mpty_hint(vc, mpty_ids);
> +
> +	n = vd->calls;
> +
> +	while (n) {
> +		nc = n ? n->data : NULL;

Why are you checking for n ? here.  Isn't while (n) above enough?

> +		if (nc && (nc->status == CALL_STATUS_ACTIVE))
> +			num_active++;

parentheses around nc->status == ... are not necessary.

Also, n->data should never be NULL.  If it is, I rather crash early.

So just checking for nc->status is sufficient.

e.g.

while (n) {
	nc = n->data;

	switch (nc->status) {
	case CALL_STATUS_ACTIVE:
		...
	case CALL_STATUS_HELD:
		...
	default:
		break;
	}
}

> +
> +		if (nc && (nc->status == CALL_STATUS_HELD))
> +			num_held++;
> +
> +		n = n->next;
> +	}
> +
> +	if ((num_active > 1 || num_held > 1) && !vd->clcc_source)
> +		vd->clcc_source = g_timeout_add(POLL_CLCC_INTERVAL, poll_clcc, vc);
>   }
>
>   static void hfp_voicecall_initialized(gboolean ok, GAtResult *result,
>

Regards,
-Denis