Hi Bertrand,
> drivers/atmodem/cbs.c | 86
> +++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/atmodem/cbs.c b/drivers/atmodem/cbs.c
> index e2e1ca4..3c250d2 100644
> --- a/drivers/atmodem/cbs.c
> +++ b/drivers/atmodem/cbs.c
> @@ -47,6 +47,8 @@ struct cbs_data {
> GAtChat *chat;
> gboolean cscb_mode_1;
> unsigned int vendor;
> + unsigned int string_limit;
> + gchar *rt; /* remaining topics */
Using rt is not a good variable name.
> };
>
> static void at_cbm_notify(GAtResult *result, gpointer user_data)
> @@ -105,6 +107,56 @@ static void at_cscb_set_cb(gboolean ok, GAtResult
> *result, gpointer user_data)
> cb(&error, cbd->data);
> }
>
> +static void at_cscb_set_cb_split(gboolean ok, GAtResult *result, gpointer
> user_data)
> +{
> + struct cb_data *cbd = user_data;
> + struct cbs_data *data = ofono_cbs_get_data(cbd->user);
> + ofono_cbs_set_cb_t cb = cbd->cb;
> + char *buf;
> + unsigned int id;
> +
> + DBG("%s", data->rt);
Where is the handling if this command responds with ERROR?
> + if (strlen(data->rt) > data->string_limit) {
> + gchar *topics_split = g_strndup(data->rt, data->string_limit);
> + gchar *new_rt;
> +
> + if (data->rt[data->string_limit] != ',') {
> + char *token = strrchr(topics_split, ',');
> + *token = 0;
> + }
> +
> + new_rt = g_strdup((char *) ((void *) data->rt +
> + strlen(topics_split) + 1));
> +
> + g_free(data->rt);
> + data->rt = new_rt;
This is code duplication. Split it our into a proper helper function.
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics_split);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> + at_cscb_set_cb_split, cbd, NULL);
> +
> + g_free(topics_split);
> + } else {
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", data->rt);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> + at_cscb_set_cb, cbd, NULL);
This is a memory leak btw. Who is freeing cbd?
> + g_free(data->rt);
> + }
> +
> + g_free(buf);
> +
> + if (id > 0)
> + return;
> +
> + g_free(cbd);
> +
> + CALLBACK_WITH_FAILURE(cb, user_data);
> +}
> +
> static void at_cbs_set_topics(struct ofono_cbs *cbs, const char *topics,
> ofono_cbs_set_cb_t cb, void *user_data)
> {
> @@ -113,7 +165,7 @@ static void at_cbs_set_topics(struct ofono_cbs *cbs,
> const char *topics,
> char *buf;
> unsigned int id;
>
> - DBG("");
> + DBG("%s", topics);
>
> /* For the Qualcomm based devices it is required to clear
> * the list of topics first. Otherwise setting the new
> @@ -128,14 +180,40 @@ static void at_cbs_set_topics(struct ofono_cbs *cbs,
> const char *topics,
> g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> NULL, NULL, NULL);
> break;
> + case OFONO_VENDOR_ZTE:
> + data->string_limit = 50;
> + g_at_chat_send(data->chat, "AT+CSCB=1", none_prefix,
> + NULL, NULL, NULL);
> + break;
Why are we copying the Qualcomm quirk over for ZTE?
> default:
> break;
> }
>
> - buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
> + if (data->string_limit && strlen(topics) > data->string_limit) {
> + gchar *topics_split = g_strndup(topics, data->string_limit);
> +
> + if (topics[data->string_limit] != ',') {
> + char *token = strrchr(topics_split, ',');
> + *token = 0;
> + }
> +
> + data->rt = g_strdup((char *) ((void *) topics +
> + strlen(topics_split) + 1));
> +
> + cbd->user = cbs;
>
> - id = g_at_chat_send(data->chat, buf, none_prefix,
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics_split);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> + at_cscb_set_cb_split, cbd, NULL);
> +
> + g_free(topics_split);
> + } else {
> + buf = g_strdup_printf("AT+CSCB=0,\"%s\"", topics);
> +
> + id = g_at_chat_send(data->chat, buf, none_prefix,
> at_cscb_set_cb, cbd, g_free);
> + }
>
> g_free(buf);
>
> @@ -240,6 +318,8 @@ static int at_cbs_probe(struct ofono_cbs *cbs, unsigned
> int vendor,
> data = g_new0(struct cbs_data, 1);
> data->chat = g_at_chat_clone(chat);
> data->vendor = vendor;
> + data->string_limit = 0;
> + data->rt = NULL;
>
> ofono_cbs_set_data(cbs, data);
>
Regards
Marcel
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
