All of lore.kernel.org
 help / color / mirror / Atom feed
From: Denis Kenzior <denkenz@gmail.com>
To: ofono@ofono.org
Subject: Re: [PATCHv3 1/5] Mobile broadband provider info plugin
Date: Fri, 12 Aug 2011 01:55:36 -0500	[thread overview]
Message-ID: <4E44CE68.7040404@gmail.com> (raw)
In-Reply-To: <1312547162-26420-2-git-send-email-oleg.zhurakivskyy@intel.com>

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

Hi Oleg,

On 08/05/2011 07:25 AM, Oleg Zhurakivskyy wrote:
> ---
>  plugins/mobile-broadband-provider-info.c |  470 ++++++++++++++++++++++++++++++
>  1 files changed, 470 insertions(+), 0 deletions(-)
>  create mode 100644 plugins/mobile-broadband-provider-info.c
> 
> diff --git a/plugins/mobile-broadband-provider-info.c b/plugins/mobile-broadband-provider-info.c
> new file mode 100644
> index 0000000..005c526
> --- /dev/null
> +++ b/plugins/mobile-broadband-provider-info.c
> @@ -0,0 +1,470 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <glib.h>
> +
> +#define OFONO_API_SUBJECT_TO_CHANGE
> +#include <ofono/types.h>
> +#include <ofono/log.h>
> +#include <ofono/plugin.h>
> +#include <ofono/modem.h>
> +#include <ofono/gprs-provision.h>
> +
> +#define _(x) case x: return (#x)
> +
> +enum element_type {
> +	ELEMENT_APN,
> +	ELEMENT_USAGE,
> +	ELEMENT_PLAN,
> +	ELEMENT_NAME,
> +	ELEMENT_NETWORK_ID,
> +	ELEMENT_USERNAME,
> +	ELEMENT_PASSWORD,
> +	ELEMENT_NONE,
> +};
> +
> +struct parser_data {
> +	struct ofono_gprs_provision_data **settings;
> +	struct ofono_gprs_provision_data *context;
> +	int count;
> +	const char *match_mcc;
> +	const char *match_mnc;
> +	enum element_type element;
> +	gboolean match_found;
> +};
> +
> +static const char *element_type_name(enum element_type element)
> +{
> +        switch (element) {
> +                _(ELEMENT_APN);
> +                _(ELEMENT_USAGE);
> +                _(ELEMENT_PLAN);
> +                _(ELEMENT_NAME);
> +                _(ELEMENT_NETWORK_ID);
> +                _(ELEMENT_USERNAME);
> +                _(ELEMENT_PASSWORD);
> +                _(ELEMENT_NONE);
> +        }
> +	return "ELEMENT_<UNKNOWN>";

Something is really funky with your whitespace, remember, tabs for
indentation, always.

> +}
> +
> +static const char *context_type_name(enum ofono_gprs_context_type context)
> +{
> +        switch (context) {
> +		_(OFONO_GPRS_CONTEXT_TYPE_ANY);
> +		_(OFONO_GPRS_CONTEXT_TYPE_INTERNET);
> +		_(OFONO_GPRS_CONTEXT_TYPE_MMS);
> +		_(OFONO_GPRS_CONTEXT_TYPE_WAP);
> +		_(OFONO_GPRS_CONTEXT_TYPE_IMS);
> +        }
> +	return "OFONO_GPRS_CONTEXT_TYPE_<UNKNOWN>";
> +}
> +
> +static const char *context_plan_name(enum ofono_gprs_context_plan plan)
> +{
> +        switch (plan) {
> +		_(OFONO_GPRS_CONTEXT_PLAN_PREPAID);
> +		_(OFONO_GPRS_CONTEXT_PLAN_POSTPAID);
> +        }
> +	return "OFONO_GPRS_CONTEXT_PLAN_<UNKNOWN>";
> +}
> +
> +static enum element_type element_type(const gchar *element)
> +{
> +	switch (element[0]) {
> +	case 'a':
> +		if (g_str_equal(element, "apn") == TRUE)
> +			return ELEMENT_APN;
> +		break;
> +	case 'n':
> +		if (g_str_equal(element, "name") == TRUE)
> +			return ELEMENT_NAME;
> +		if (g_str_equal(element, "network-id") == TRUE)
> +			return ELEMENT_NETWORK_ID;
> +		break;
> +	case 'p':
> +		if (g_str_equal(element, "plan") == TRUE)
> +			return ELEMENT_PLAN;
> +		if (g_str_equal(element, "password") == TRUE)
> +			return ELEMENT_PASSWORD;
> +		break;
> +	case 'u':
> +		if (g_str_equal(element, "usage") == TRUE)
> +			return ELEMENT_USAGE;
> +		if (g_str_equal(element, "username") == TRUE)
> +			return ELEMENT_USERNAME;
> +	}
> +
> +	return ELEMENT_NONE;
> +}
> +
> +static void element_network_id_parse(struct parser_data *data,
> +					const gchar **attribute_names,
> +					const gchar **attribute_values)
> +{
> +	const char *mcc = NULL, *mnc = NULL;
> +	int i;
> +
> +	for (i = 0; attribute_names[i]; i++) {
> +		if (g_str_equal(attribute_names[i], "mcc") == TRUE)
> +			mcc = attribute_values[i];
> +		if (g_str_equal(attribute_names[i], "mnc") == TRUE)
> +			mnc = attribute_values[i];
> +	}
> +
> +	if (g_strcmp0(mcc, data->match_mcc) == 0 &&
> +			g_strcmp0(mnc, data->match_mnc) == 0)
> +		data->match_found = TRUE;
> +}
> +
> +static struct ofono_gprs_provision_data *apn_try_new(struct parser_data *data)
> +{
> +	void *p;
> +
> +	p = g_try_realloc(*data->settings,
> +				sizeof(struct ofono_gprs_provision_data) *
> +				(data->count + 1));
> +	if (p == NULL)
> +		return NULL;
> +
> +	*data->settings = p;
> +
> +	data->context = *data->settings + data->count++;
> +
> +	memset(data->context, 0, sizeof(struct ofono_gprs_provision_data));

This seems like a really big hammer.  We have 3 settings we're likely to
provision, internet, wap, mms.  The function runs synchronously and
oFono is not threaded.  So in theory you can allocate the context
information statically and only copy it to the caller once you have
completed the parser.

> +
> +	return data->context;
> +}
> +
> +static void element_apn_parse(struct parser_data *data,
> +				const gchar **attribute_names,
> +				const gchar **attribute_values)
> +{
> +	const char *apn = NULL;
> +	int i;
> +
> +	for (i = 0; attribute_names[i]; i++) {
> +		if (g_str_equal(attribute_names[i], "value") == TRUE)
> +			apn = attribute_values[i];
> +	}
> +
> +	if (apn_try_new(data) == NULL) {
> +		data->match_found = FALSE;
> +		ofono_error("APN '%s' is ignored, memory exhausted", apn);
> +		return;
> +	}
> +
> +	data->context->apn = g_strdup(apn);
> +}
> +
> +static void element_usage_parse(struct ofono_gprs_provision_data *context,
> +				const gchar **attribute_names,
> +				const gchar **attribute_values)
> +{
> +	const char *usage = NULL;
> +	int i;
> +
> +	for (i = 0; attribute_names[i]; i++) {
> +		if (g_str_equal(attribute_names[i], "type") == TRUE)
> +			usage = attribute_values[i];
> +	}
> +
> +	if (usage == NULL)
> +		return;
> +
> +	if (g_str_equal(usage, "internet") == TRUE)
> +		context->type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
> +	else if (g_str_equal(usage, "mms") == TRUE)
> +		context->type = OFONO_GPRS_CONTEXT_TYPE_MMS;
> +	else if (g_str_equal(usage, "wap") == TRUE)
> +		context->type = OFONO_GPRS_CONTEXT_TYPE_WAP;
> +}
> +
> +static void element_plan_parse(struct ofono_gprs_provision_data *context,
> +				const gchar **attribute_names,
> +				const gchar **attribute_values)
> +{
> +	const char *plan = NULL;
> +	int i;
> +
> +	for (i = 0; attribute_names[i]; i++) {
> +		if (g_str_equal(attribute_names[i], "type") == TRUE)
> +			plan = attribute_values[i];
> +	}
> +
> +	if (plan == NULL)
> +		return;
> +
> +	if (g_str_equal(plan, "postpaid") == TRUE)
> +		context->plan = OFONO_GPRS_CONTEXT_PLAN_POSTPAID;
> +	else if (g_str_equal(plan, "prepaid") == TRUE)
> +		context->plan = OFONO_GPRS_CONTEXT_PLAN_PREPAID;
> +}
> +
> +static void element_start_parse(GMarkupParseContext *context,
> +				const gchar *element_name,
> +				const gchar **attribute_names,
> +				const gchar **attribute_values,
> +				gpointer user_data, GError **error)
> +{
> +	struct parser_data *data = user_data;
> +
> +	data->element = element_type(element_name);
> +
> +	if (data->match_found == TRUE)
> +		DBG("%s: '%s'", element_type_name(data->element), element_name);
> +
> +	if (data->element == ELEMENT_NETWORK_ID)
> +		element_network_id_parse(data, attribute_names,
> +						attribute_values);
> +	if (data->match_found == FALSE)
> +		return;
> +
> +	switch (data->element) {
> +	case ELEMENT_APN:
> +		element_apn_parse(data, attribute_names, attribute_values);
> +		break;
> +	case ELEMENT_USAGE:
> +		element_usage_parse(data->context, attribute_names,
> +					attribute_values);
> +		break;
> +	case ELEMENT_PLAN:
> +		element_plan_parse(data->context, attribute_names,
> +					attribute_values);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +static gchar *body_text_parse(const gchar *text, gsize text_len)
> +{
> +	gchar *body = g_strndup(text, text_len);
> +	gchar *print = NULL;
> +	int i;
> +
> +	for (i = 0; body && body[i]; i++) {
> +		if (g_ascii_isprint(body[i])) {
> +			print = g_strescape(body, NULL);
> +			break;
> +		}
> +	}
> +
> +	if (body)
> +		g_free(body);
> +
> +	return print;
> +}
> +
> +static void element_body_parse(GMarkupParseContext *context,
> +				const gchar *text, gsize text_len,
> +				gpointer user_data, GError **error)
> +{
> +	struct parser_data *data = user_data;
> +	gchar *body;
> +
> +	if (data->match_found == FALSE || text_len == 0)
> +		return;
> +
> +	body = body_text_parse(text, text_len);
> +	if (body == NULL)
> +		return;
> +
> +	DBG("%s: '%s'", element_type_name(data->element), body);
> +
> +	switch (data->element) {
> +	case ELEMENT_USERNAME:
> +		data->context->username = g_strdup(body);
> +		break;
> +	case ELEMENT_PASSWORD:
> +		data->context->password = g_strdup(body);
> +		break;
> +	case ELEMENT_NAME:
> +		data->context->name = g_strdup(body);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	g_free(body);
> +}
> +
> +static void element_end_parse(GMarkupParseContext *context,
> +				const gchar *element_name,
> +				gpointer user_data, GError **error)
> +{
> +	struct parser_data *data = user_data;
> +
> +	if (data->match_found == FALSE)
> +		return;
> +
> +	DBG("%s: '%s'", element_type_name(data->element), element_name);
> +
> +	if (g_str_equal(element_name, "gsm") == TRUE)
> +		data->match_found = FALSE;
> +}
> +

Maybe I'm just not seeing it, but how are we handling duplicates?  E.g.
the case where mcc/mncs match but correspond to two different operators.
 Or where we find multiple matches of the same context type.

> +static void parser_error(GMarkupParseContext *context,
> +				GError *error, gpointer user_data)
> +{
> +	ofono_error("Error parsing %s: %s", PROVIDER_DATABASE, error->message);
> +}
> +
> +static const GMarkupParser parser = {
> +	element_start_parse,
> +	element_end_parse,
> +	element_body_parse,
> +	NULL,
> +	parser_error,
> +};
> +
> +static void parse_database(const char *data, ssize_t size,
> +	struct parser_data *parser_data)
> +{
> +	GMarkupParseContext *context;
> +	gboolean result;
> +
> +	parser_data->match_found = FALSE;
> +
> +	context = g_markup_parse_context_new(&parser,
> +						G_MARKUP_TREAT_CDATA_AS_TEXT,
> +						parser_data, NULL);
> +
> +	result = g_markup_parse_context_parse(context, data, size, NULL);
> +	if (result == TRUE)
> +		result = g_markup_parse_context_end_parse(context, NULL);
> +
> +	g_markup_parse_context_free(context);
> +}
> +
> +static int lookup_apn(const char *mcc, const char *mnc, const char *spn,
> +			struct parser_data *data)
> +{
> +	struct stat st;
> +	char *map;
> +	int fd;
> +
> +	if (mcc == NULL || mnc == NULL)
> +		return -1;
> +
> +	fd = open(PROVIDER_DATABASE, O_RDONLY);
> +	if (fd < 0)
> +		return -1;
> +
> +	if (fstat(fd, &st) < 0) {
> +		close(fd);
> +		return -1;
> +	}
> +
> +	map = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
> +	if (map == NULL || map == MAP_FAILED) {

man mmap tells me that MAP_FAILED is the only error condition. Why are
we checking for NULL as well?

> +		close(fd);
> +		return -1;
> +	}
> +
> +	data->match_mcc = mcc;
> +	data->match_mnc = mnc;
> +
> +	parse_database(map, st.st_size, data);
> +
> +	munmap(map, st.st_size);
> +
> +	close(fd);
> +
> +	return data->count;
> +}
> +
> +static int mobile_broadband_provider_info_get(const char *mcc,
> +				const char *mnc, const char *spn,
> +				struct ofono_gprs_provision_data **settings,
> +				int *count)
> +{
> +	struct parser_data *data;
> +	int i;
> +
> +	DBG("Provisioning for MCC %s, MNC %s, SPN '%s'", mcc, mnc, spn);
> +
> +	*settings = NULL;
> +	*count = 0;

In general I like not to modify the output args unless the function is
on the success path.

> +
> +	data = g_try_new0(struct parser_data, 1);
> +	if (data == NULL)
> +		return -ENOMEM;
> +

As mentioned before the parser is running synchronously, so this could
actually be statically allocated.

> +	data->settings = settings;
> +
> +	*count = lookup_apn(mcc, mnc, NULL, data);
> +	if (*count <= 0) {
> +		g_free(data);
> +		*settings = NULL;
> +		return -ENOENT;
> +	}
> +
> +	DBG("settings: %p, count: %d", *settings, *count);
> +
> +	for (i = 0; i < *count; i++) {
> +		struct ofono_gprs_provision_data *context = *settings + i;
> +		DBG("Name: '%s'", context->name);
> +		DBG("APN: '%s'", context->apn);
> +		DBG("Type: %s", context_type_name(context->type));
> +		DBG("Plan: %s", context_plan_name(context->plan));
> +		DBG("Username: '%s'", context->username);
> +		DBG("Password: '%s'", context->password);
> +	}
> +
> +	g_free(data);
> +
> +	return 0;
> +}
> +
> +static struct ofono_gprs_provision_driver provider_info = {
> +	.name		= "Mobile Broadband Provider Info",
> +	.get_settings	= mobile_broadband_provider_info_get
> +};
> +
> +static int mobile_broadband_provider_info_init(void)
> +{
> +	return ofono_gprs_provision_driver_register(&provider_info);
> +}
> +
> +static void mobile_broadband_provider_info_exit(void)
> +{
> +	ofono_gprs_provision_driver_unregister(&provider_info);
> +}
> +
> +OFONO_PLUGIN_DEFINE(mobile_broadband_provider_info,
> +			"Mobile Broadband Provider Info Plugin",
> +			VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
> +			mobile_broadband_provider_info_init,
> +			mobile_broadband_provider_info_exit)

Regards,
-Denis

  reply	other threads:[~2011-08-12  6:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-05 12:25 [PATCHv3 0/5] Mobile broadband provider info plugin Oleg Zhurakivskyy
2011-08-05 12:25 ` [PATCHv3 1/5] " Oleg Zhurakivskyy
2011-08-12  6:55   ` Denis Kenzior [this message]
2011-08-16 14:11     ` Oleg Zhurakivskyy
2011-08-18 18:37       ` Denis Kenzior
2011-08-05 12:25 ` [PATCHv3 2/5] GPRS context plan support Oleg Zhurakivskyy
2011-08-12  6:43   ` Denis Kenzior
2011-08-05 12:26 ` [PATCHv3 3/5] GPRS provisioning data " Oleg Zhurakivskyy
2011-08-05 12:26 ` [PATCHv3 4/5] Mobile broadband provider info plugin autoconf support Oleg Zhurakivskyy
2011-08-05 12:26 ` [PATCHv3 5/5] Mobile broadband provider info plugin makefile changes Oleg Zhurakivskyy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E44CE68.7040404@gmail.com \
    --to=denkenz@gmail.com \
    --cc=ofono@ofono.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.