Re: [PATCH 5/5] atmodem: sim-auth atom driver.

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Andrew,

> +static void at_discover_apps_cb(gboolean ok, GAtResult *result,
> +				gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	GAtResultIter iter;
> +	ofono_sim_list_apps_cb_t cb = cbd->cb;
> +	struct ofono_error error;
> +	GSList *list = NULL, *i;
> +	struct ofono_sim_app_record *buffer;
> +	int j, n;
> +
> +	decode_at_error(&error, g_at_result_final_response(result));
> +
> +	if (!ok) {
> +		cb(&error, NULL, 0, cbd->data);
> +		return;
> +	}
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	while (g_at_result_iter_next(&iter, "+CUAD:")) {
> +		const guint8 *obj_str;
> +		gint len;
> +
> +		if (!g_at_result_iter_next_hexstring(&iter, &obj_str, &len))
> +			goto error;
> +
> +		i = sim_parse_app_template_entries(obj_str, len);
> +		if (i == NULL)
> +			goto error;
> +
> +		list = g_slist_concat(i, list);
> +	}
> +

Since we potentially multiple results, what do you think of using an
approach similar to how phonebook works.  E.g. a simple:

typedef void (*ofono_sim_auth_cb_t)(const struct ofono_error *error,
                                        void *data);

and ofono_sim_auth_app_record(struct ofono_sim_auth *sa, unsigned char
*record, int len);

This would work nicely even for drivers using CRSM I think...

> +	n = g_slist_length(list);
> +	buffer = g_new(struct ofono_sim_app_record, n);
> +
> +	for (i = list, j = n - 1; i != NULL; i = i->next, i--)
> +		memcpy(&buffer[j], i->data, sizeof(*buffer));
> +
> +	cb(&error, buffer, n, cbd->data);
> +
> +	g_free(buffer);
> +	goto done;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cb, NULL, 0, cbd->data);
> +
> +done:
> +	while (list) {
> +		i = list;
> +		list = list->next;
> +
> +		g_free(((struct ofono_sim_app_record *) i->data)->label);
> +		g_free(i->data);
> +		g_slist_free_1(i);
> +	}
> +}

Regards,
-Denis