[PATCH] watch: Free service data in service_reply
Avoid the memory leak of server_data.
---
gdbus/watch.c |5 -
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 1d479fa..ccdbb64 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -468,8 +468,10 @@ static void service_reply(DBusPendingCall *call, void
*user_data)
dbus_bool_t has_owner;
reply = dbus_pending_call_steal_reply(call);
- if (reply == NULL)
+ if (reply == NULL) {
+ g_free(data);
return;
+ }
dbus_error_init(error);
@@ -490,6 +492,7 @@ static void service_reply(DBusPendingCall *call, void
*user_data)
done:
dbus_message_unref(reply);
+ g_free(data);
}
static void check_service(DBusConnection *connection, const char *name,
--
1.7.0.4
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[PATCH 1/5] bluetooth: Add reference count for bluetooth utils
Add bluetooth_ref()/bluetooth_unref() to support reference count in
bluetooth utils.
---
plugins/bluetooth.c | 62 +--
1 files changed, 45 insertions(+), 17 deletions(-)
diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
index 5a85eaa..10cc49d 100644
--- a/plugins/bluetooth.c
+++ b/plugins/bluetooth.c
@@ -39,6 +39,7 @@
static DBusConnection *connection;
static GHashTable *uuid_hash = NULL;
static GHashTable *adapter_address_hash = NULL;
+static gint ref_count;
void bluetooth_create_path(const char *dev_addr, const char *adapter_addr,
char *buf, int size)
@@ -503,13 +504,10 @@ static guint adapter_added_watch;
static guint adapter_removed_watch;
static guint property_watch;
-int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile
*profile)
+static int bluetooth_init()
{
int err;
- if (uuid_hash)
- goto done;
-
connection = ofono_dbus_get_connection();
bluetooth_watch = g_dbus_add_service_watch(connection, BLUEZ_SERVICE,
@@ -542,13 +540,6 @@ int bluetooth_register_uuid(const char *uuid, struct
bluetooth_profile *profile)
adapter_address_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, g_free);
-done:
- g_hash_table_insert(uuid_hash, g_strdup(uuid), profile);
-
- bluetooth_send_with_reply(/, BLUEZ_MANAGER_INTERFACE, GetProperties,
- manager_properties_cb, NULL, NULL, -1,
- DBUS_TYPE_INVALID);
-
return 0;
remove:
@@ -556,14 +547,27 @@ remove:
g_dbus_remove_watch(connection, adapter_added_watch);
g_dbus_remove_watch(connection, adapter_removed_watch);
g_dbus_remove_watch(connection, property_watch);
+
return err;
}
-void bluetooth_unregister_uuid(const char *uuid)
+static int bluetooth_ref()
{
- g_hash_table_remove(uuid_hash, uuid);
+ g_atomic_int_inc(ref_count);
+
+ if (ref_count 1)
+ return 0;
+
+ return bluetooth_init();
+}
+
+static void bluetooth_unref()
+{
+ gboolean is_zero;
+
+ is_zero = g_atomic_int_dec_and_test(ref_count);
- if (g_hash_table_size(uuid_hash))
+ if (is_zero == FALSE)
return;
g_dbus_remove_watch(connection, bluetooth_watch);
@@ -571,9 +575,33 @@ void bluetooth_unregister_uuid(const char *uuid)
g_dbus_remove_watch(connection, adapter_removed_watch);
g_dbus_remove_watch(connection, property_watch);
- g_hash_table_destroy(uuid_hash);
- g_hash_table_destroy(adapter_address_hash);
- uuid_hash = NULL;
+ if (uuid_hash)
+ g_hash_table_destroy(uuid_hash);
+
+ if (adapter_address_hash)
+ g_hash_table_destroy(adapter_address_hash);
+}
+
+int bluetooth_register_uuid(const char *uuid, struct bluetooth_profile
*profile)
+{
+ int err = bluetooth_ref();
+
+ if (err != 0)
+ return err;
+
+ g_hash_table_insert(uuid_hash, g_strdup(uuid), profile);
+
+ bluetooth_send_with_reply(/, BLUEZ_MANAGER_INTERFACE, GetProperties,
+ manager_properties_cb, NULL, NULL, -1,
+ DBUS_TYPE_INVALID);
+ return 0;
+}
+
+void bluetooth_unregister_uuid(const char *uuid)
+{
+ g_hash_table_remove(uuid_hash, uuid);
+
+ bluetooth_unref();
}
OFONO_PLUGIN_DEFINE(bluetooth, Bluetooth Utils Plugins, VERSION,
--
1.7.0.4
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[PATCH 0/5] Add oFono DUN emulator
Hi, These are the updated patches for DUN emulator. First, it listens modem Online signal to register itself to bluetooth library. Second, it register DUN record through D-Bus interface. Btio library from Obex is simplified to only keep RFCOMM part. Thanks, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH 2/5] bluetooth: Add bluetooth server support for DUN
It watches Bluetooth adapter property changes and added DUN record to
listen client connection request. Btio library is the low level socket
API for RFCOMM connection.
---
plugins/bluetooth.c | 379
plugins/bluetooth.h | 14 ++
plugins/btio.c | 480 +++
plugins/btio.h | 118 +
4 files changed, 991 insertions(+), 0 deletions(-)
create mode 100644 plugins/btio.c
create mode 100644 plugins/btio.h
diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
index 10cc49d..5890161 100644
--- a/plugins/bluetooth.c
+++ b/plugins/bluetooth.c
@@ -35,12 +35,71 @@
#include ofono/dbus.h
#include bluetooth.h
+#include btio.h
static DBusConnection *connection;
static GHashTable *uuid_hash = NULL;
static GHashTable *adapter_address_hash = NULL;
+static GSList *server_list = NULL;
static gint ref_count;
+static const gchar *dun_record = ?xml version=\1.0\ encoding=\UTF-8\ ?
\
+record
\
+ attribute id=\0x0001\
\
+sequence
\
+ uuid value=\0x1103\/
\
+/sequence
\
+ /attribute
\
+
\
+ attribute id=\0x0004\
\
+sequence
\
+ sequence
\
+uuid value=\0x0100\/
\
+ /sequence
\
+ sequence
\
+uuid value=\0x0003\/
\
+uint8 value=\%u\ name=\channel\/
\
+ /sequence
\
+ sequence
\
+uuid value=\0x0008\/
\
+ /sequence
\
+/sequence
\
+ /attribute
\
+
\
+ attribute id=\0x0009\
\
+sequence
\
+ sequence
\
+uuid value=\0x1103\/
\
+uint16 value=\0x0100\ name=\version\/
\
+ /sequence
\
+/sequence
\
+ /attribute
\
+
\
+ attribute id=\0x0100\
\
+text value=\%s\ name=\name\/
\
+ /attribute
\
+/record;
+
+#define TIMEOUT (60*1000) /* Timeout for user response (miliseconds) */
+
+struct client {
+ GIOChannel *io;
+ guint watch;
+};
+
+struct server {
+ guint16 service;
+ gchar *name;
+ guint8 channel;
+ GIOChannel *io;
+ gchar *adapter;
+ guint handle;
+ ConnectFunc connect_cb;
+ gpointeruser_data;
+
+ struct client client;
+};
+
void bluetooth_create_path(const char *dev_addr, const char *adapter_addr,
char *buf, int size)
{
@@ -370,6 +429,253 @@ static gboolean property_changed(DBusConnection
*connection, DBusMessage *msg,
return TRUE;
}
+static void disconnect(struct server *server)
+{
+ struct client *client = server-client;
+
+ if (!client-io)
+ return;
+
+ g_io_channel_unref(client-io);
+ client-io = NULL;
+
+ if (client-watch 0) {
+ g_source_remove(client-watch);
+ client-watch = 0;
+ }
+
+ return;
+}
+
+static void server_stop(gpointer data, gpointer user_data)
+{
+ struct server *server = data;
+
+ disconnect(server);
+
[PATCH 3/5] modem: Add method to get modem by path
Return modem instance by searching modem path.
---
include/modem.h |1 +
src/modem.c | 15 +++
2 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/include/modem.h b/include/modem.h
index e1cd049..34c3fbf 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -36,6 +36,7 @@ void ofono_modem_remove_interface(struct ofono_modem *modem,
const char *interface);
const char *ofono_modem_get_path(struct ofono_modem *modem);
+struct ofono_modem *ofono_modem_get_modem_by_path(const char *path);
void ofono_modem_set_data(struct ofono_modem *modem, void *data);
void *ofono_modem_get_data(struct ofono_modem *modem);
diff --git a/src/modem.c b/src/modem.c
index f89d609..6907a5e 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -157,6 +157,21 @@ const char *ofono_modem_get_path(struct ofono_modem *modem)
return NULL;
}
+struct ofono_modem *ofono_modem_get_modem_by_path(const char *path)
+{
+ GSList *l;
+ struct ofono_modem *modem = NULL;
+
+ for (l = g_modem_list; l; l = l-next) {
+ modem = l-data;
+
+ if (g_str_equal(modem-path, path))
+ break;
+ }
+
+ return modem;
+}
+
struct ofono_atom *__ofono_modem_add_atom(struct ofono_modem *modem,
enum ofono_atom_type type,
void (*destruct)(struct ofono_atom *),
--
1.7.0.4
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[PATCH 5/5] emulator: Add emulator atom in oFono
DUN server could create emulator and use GAtServer to talk AT commands
to DUN client side.
---
Makefile.am|4 +-
include/emulator.h | 41 ++
plugins/dun_gw.c |8 +++
src/emulator.c | 121
src/ofono.h|3 +
5 files changed, 175 insertions(+), 2 deletions(-)
create mode 100644 include/emulator.h
create mode 100644 src/emulator.c
diff --git a/Makefile.am b/Makefile.am
index 2e08ff2..1ec04f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -13,7 +13,7 @@ include_HEADERS = include/log.h include/plugin.h
include/history.h \
include/cbs.h include/call-volume.h \
include/gprs.h include/gprs-context.h \
include/radio-settings.h include/stk.h \
- include/nettime.h
+ include/nettime.h include/emulator.h
nodist_include_HEADERS = include/version.h
@@ -274,7 +274,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) \
src/storage.c src/cbs.c src/watch.c src/call-volume.c \
src/gprs.c src/idmap.h src/idmap.c \
src/radio-settings.c src/stkutil.h src/stkutil.c \
- src/nettime.c
+ src/nettime.c src/emulator.c
src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
diff --git a/include/emulator.h b/include/emulator.h
new file mode 100644
index 000..1033e59
--- /dev/null
+++ b/include/emulator.h
@@ -0,0 +1,41 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2010 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
+ *
+ */
+
+#ifndef __OFONO_EMULATOR_H
+#define __OFONO_EMULATOR_H
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+#include ofono/types.h
+
+struct ofono_emulator;
+
+struct ofono_emulator *ofono_emulator_create(struct ofono_modem *modem,
+ enum ofono_atom_type type,
+ GIOChannel *io);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_EMULATOR_H */
diff --git a/plugins/dun_gw.c b/plugins/dun_gw.c
index 56bd03d..840d22d 100644
--- a/plugins/dun_gw.c
+++ b/plugins/dun_gw.c
@@ -48,6 +48,9 @@ static int modem_watch;
static void dun_gw_connect_cb(GIOChannel *io, GError *err, gpointer user_data)
{
+ struct ofono_modem *modem = user_data;
+ struct ofono_emulator *emulator;
+
DBG();
if (err) {
@@ -55,6 +58,11 @@ static void dun_gw_connect_cb(GIOChannel *io, GError *err,
gpointer user_data)
goto failed;
}
+ emulator = ofono_emulator_create(modem, OFONO_ATOM_TYPE_EMULATOR_DUN,
+ io);
+ if (!emulator)
+ goto failed;
+
return;
failed:
diff --git a/src/emulator.c b/src/emulator.c
new file mode 100644
index 000..6219bb1
--- /dev/null
+++ b/src/emulator.c
@@ -0,0 +1,121 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2010 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 stdio.h
+#include string.h
+#include errno.h
+#include glib.h
+#include gdbus.h
+
+#include ofono.h
+#include common.h
+#include gatserver.h
+
+struct ofono_emulator {
+ struct ofono_modem *modem;
+ struct ofono_atom *atom;
+ enum ofono_atom_type type;
+ unsigned int id;
+ GAtServer *server;
+};
+
+static unsigned int ofono_emulator_ids;
+
+static void ofono_emulator_debug(const char *str, void *data)
+{
+
RE: Howto about Qt QML and Ofono
Hi Matthias, Matthias Günther wrote: Hi Yang, http://matgnt.wordpress.com/2010/07/19/qt-ofono-d-bus-and-qml-part-1/ http://matgnt.wordpress.com/2010/07/19/qt-ofono-d-bus-and-qml-part-2/ http://matgnt.wordpress.com/2010/07/19/qt-ofono-d-bus-and-qml-part-3/ I hope this helps people out there to create cool new phone applications. You're welcome to leave a note here or on the howto directly. Your lovely program gives a good demo on how to writing an application based on oFono. oFono does claim it will help on building application in a convenient way, so the prosperity of application will definitely bring feedback to oFono and make oFono grow better. I have two comments on your first part: 1) configure is not in the git tree of oFono, so we will use bootstrap-configure instead. 2) I'm sure your phonesim works well. But we also have another phonesim derived from your repo, which we had some modification and enhancement in. The repo is git://git.kernel.org/pub/scm/network/ofono/phonesim.git. The building command is same as oFono, and the execution command is ./phonesim -p 12345 -gui src/default.xml. Thanks Yang, I've added the ./bootstrap command to the listing. I also wrote down a hint to the ofono branch of phonesim. bootstrap-configure is a single script, rather than two different ones. Thanks for your comment. As mentioned in the thread before, there are two scripts. The problem with bootstrap-configure is, that it enables the --enable-capng switch which may cause problems. It'll probably end up in missing files. Therefore I chose bootstrap to just create the configure script and then use it without any configuration arguments - just the defaults. For the purpose of the howto, the debug switches are not really necessary. You could use 'sudo apt-get install libcap-ng-dev' or 'yum install -y libcap-ng-devel' to install this library. One comment is that we normally call our project as 'oFono' instead 'Ofono'. :-) Best regards, Matthias ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] watch: Free service data in service_reply
Hi Zhenhua, Avoid the memory leak of server_data. --- gdbus/watch.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) do me a favor and post this to linux-blueto...@vger.kernel.org as well for the Bluetooth guys to review. After that I can take of applying it to all trees. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/5] stk: Add menu related utilities.
Hi Andrew,
+static struct stk_menu *stk_menu_create(const char *title,
+ const struct stk_text_attribute *title_attr, GSList *items,
+ const struct stk_item_text_attribute_list *item_attrs,
+ const struct stk_items_next_action_indicator *next_action,
+ int default_id, gboolean soft_key, gboolean has_help)
+{
I suggest breaking up this giant constructor with many arguments into
two functions. One that takes a setup_menu object and one that takes a
select_item object. Then fills in the items appropriately.
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] stkutil: convert img to xpm
Hi Kristen,
On 07/22/2010 07:10 PM, Kristen Carlson Accardi wrote:
---
src/stkutil.c | 148
+
src/stkutil.h |8 +++
2 files changed, 156 insertions(+), 0 deletions(-)
diff --git a/src/stkutil.c b/src/stkutil.c
index 9cac850..46ae026 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -6076,3 +6076,151 @@ char *stk_text_to_html(const char *utf8,
/* return characters from string. Caller must free char data */
return g_string_free(string, FALSE);
}
+
+static const char chars_table[] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'A', 'B', 'C',
+ 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P',
+ 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', 'a', 'b', 'c',
+ 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p',
+ 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z', '+', '.' };
+
+char *stk_image_to_xpm(const unsigned char *img, unsigned int len,
+ enum stk_img_scheme scheme)
+{
+ guint8 width, height;
+ unsigned int ncolors, nbits, entry, cpp;
+ unsigned int i, j;
+ int bit, k;
+ unsigned short clut_offset = 0;
+ guint8 *clut;
+ GString *xpm;
+ unsigned int pos = 0;
+ const char xpm_header[] = /* XPM */\n;
+ const char declaration[] = static char *xpm[] = {\n;
+ char c[3];
+
+ if (img == NULL)
+ return NULL;
+
+ /* sanity check length */
+ if (len 3)
+ return NULL;
+
+ width = img[pos++];
+ height = img[pos++];
+
+ if (scheme == STK_IMG_SCHEME_BASIC) {
+ nbits = 1;
+ ncolors = 2;
+
+ if (pos + ((width * height)/8) len)
+ return NULL;
You probably want (pos + (width * height + 7) / 8) len here. Also,
please keep doc/coding-style.txt Section M3 in mind here.
+ } else {
+ /* sanity check length */
+ if (pos + 4 len)
+ return NULL;
+
+ nbits = img[pos++];
+ ncolors = img[pos++];
+
+ /* the value of zero should be interpreted as 256 */
+ if (ncolors == 0)
+ ncolors = 256;
+
+ clut_offset = img[pos++] 8;
+ clut_offset |= img[pos++];
+
+ if ((clut_offset + (ncolors * 3) len) ||
+ (pos + (width * height * nbits)/8) clut_offset)
Please keep in mind doc/coding-style.txt Section M4. Sometimes it is
better to separate the || conditions into two separate ifs to satisfy
this rule.
Also, you're off by 1 again here. You probably want (width * height *
nbits + 7) / 8.
This also brings up another point. You're assuming that the caller is
appending the CLUT right after the image data and massaging the clut
offset appropriately. This is a really bad idea since the caller will
have to do some significant pre-processing.
You can handle this in one of two ways:
1. Assume the calling logic will read the entire image file before
calling this function. In this case, modifying the signature as follows
might be a good idea:
char *stk_image_to_xpm(const unsigned char *file,
unsigned short file_len,
enum stk_img_scheme scheme,
unsigned short img_offset,
unsigned short img_len);
2. Assume the calling logic is clever and will optimize reading of the
file, including peeking into the image header to determine the where the
CLUT is located and reading it. In this case you can either reuse the
signature from 1 above, or come up with something else.
Remember, reading from the SIM is extremely slow, so any and all clever
optimization tricks are definitely wanted.
+ return NULL;
+ }
+
+ /* determine the number of chars need to represent the pixel */
+ cpp = ncolors 64 ? 2 : 1;
+
+ /*
+ * space needed:
+ * header line
+ * declaration and beginning of assignment line
+ * values - max length of 19
+ * colors - ncolors * (cpp + whitespace + deliminators + color)
+ * pixels - width * height * cpp + height deliminators ,\n
+ * end of assignment - 2 chars };
+ */
+ xpm = g_string_sized_new(strlen(xpm_header) + strlen(declaration) +
+ 19 + ((cpp + 14) * ncolors) +
+ (width * height * cpp) + (4 * height) + 2);
+ if (xpm == NULL)
+ return NULL;
+
+ /* add header, declaration, values */
+ g_string_append(xpm, xpm_header);
+ g_string_append(xpm, declaration);
+ g_string_append_printf(xpm, \%d %d %d %d\,\n, width, height,
+ ncolors, cpp);
+
+ /* create colors */
+ if (scheme == STK_IMG_SCHEME_BASIC) {
+ g_string_append(xpm, \0\tc
[patch 01/20] bug.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
These have been stolen from the Linux kernel source; come pretty handy
to make build-time consistency checks and thus avoid run-time
surprises.
---
src/bug.h | 50 ++
src/smsutil.c |1 +
2 files changed, 51 insertions(+), 0 deletions(-)
create mode 100644 src/bug.h
diff --git a/src/bug.h b/src/bug.h
new file mode 100644
index 000..07c14b9
--- /dev/null
+++ b/src/bug.h
@@ -0,0 +1,50 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2010 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
+ *
+ */
+
+/*
+ * Build time consistency checks
+ *
+ * Copied from the Linux kernel, linux/include/linux/kernel.h
+ * v2.6.35-rc; this file has no explicit license header, thus is
+ * covered by linux/COPYING which licenses it as GPL v2 only.
+ */
+
+/* Force a compilation error if condition is true */
+#define BUILD_BUG_ON(condition) ((void)BUILD_BUG_ON_ZERO(condition))
+
+/* Force a compilation error if condition is constant and true */
+#define MAYBE_BUILD_BUG_ON(cond) ((void)sizeof(char[1 - 2 * !!(cond)]))
+
+/* Force a compilation error if a constant expression is not a power of 2 */
+#define BUILD_BUG_ON_NOT_POWER_OF_2(n) \
+ BUILD_BUG_ON((n) == 0 || (((n) ((n) - 1)) != 0))
+
+/* Force a compilation error if condition is true, but also produce a
+ result (of value 0 and type size_t), so the expression can be used
+ e.g. in a structure initializer (or where-ever else comma expressions
+ aren't permitted). */
+#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
+#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+
+/*
+ * End of code copied from linux/include/linux/kernel.h
+ * v2.6.35-rc.
+ */
diff --git a/src/smsutil.c b/src/smsutil.c
index e41c041..6c2087a 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -37,6 +37,7 @@
#include util.h
#include storage.h
#include smsutil.h
+#include bug.h
#define uninitialized_var(x) x = x
--
1.6.6.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[patch 00/20] SMS D-Bus support and misc small patches
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com This (3rd? 4th?) version of the patchset builds the D-Bus support on top of the new _txq_submit() callback mechanism. Note there are still a couple of opens that need discussion: - the message ID is generated based on the contents of the message -- thus, the current way doesn't work. We need the caller to _txq_submit() to generate it. It's been left out of the STK stc.c:handle_command_send_sms() because I am not sure what is the right way to do it -- need feedback on that. - The generation of the SMS message ID based on contents still has shortcomings: if we submit two messages with the same content and destination number, the ID is the same [sms.c:sms_msg_send()]. What other factor would make sense to add? time? The following changes since commit 94344e967b4cd3edd65aa5254ef4b4f5dd037e69: Denis Kenzior (1): TODO: Major updates to STK related tasks are available in the git repository at: git://gitorious.org/~inakypg/ofono/ofono-inakypg.git master Patches follow for reviewing convenience. Inaky Perez-Gonzalez (20): bug.h: Add BUILD_BUG_ON() and friends for compile-time assert checking write_file: make transaction-safe manpage: explain debugging options to -d SMS: introduce message ID API introduce DECLARE_SMS_ADDR_STR() _assembly_encode_address: export and rename SMS: implement SHA256-based message IDs [incomplete] sms: document the org.ofono.SMSMessage D-Bus interface SMS: document handle_sms_status_report() sms_text_prepare: document @use_delivery_reports SMS: rename create_tx_queue_entry() to tx_queue_entry_new() struct tx_queue_entry: add a destructor SMS: encapsulate D-Bus specific data in 'struct sms_msg_dbus_data' SMS: introduce bare state machine and transitions SMS: introduce Wait-for-Status-Report state and infrastructure SMS: introduce a state change callback for TX messages SMS: export outgoing messages over D-Bus SMS: send D-Bus SMS-MSG::PropertyChanged signals when message changes status SMS: introduce sms_msg_cancel and its D-Bus wrapper SMS: Implement D-Bus SMS-MSG::GetProperties HACKING| 10 + Makefile.am|5 +- doc/ofonod.8 |5 +- doc/sms-api.txt| 49 - src/bug.h | 50 src/ofono.h| 42 +++- src/sms.c | 598 ++-- src/smsutil.c | 206 ++- src/smsutil.h | 122 src/stk.c | 24 ++- src/storage.c | 42 ++- test/test-sms-msg-cancel | 173 test/test-sms-msg-state-change | 24 ++ unit/test-sms-msg-id.c | 212 ++ 14 files changed, 1449 insertions(+), 113 deletions(-) create mode 100644 src/bug.h create mode 100755 test/test-sms-msg-cancel create mode 100755 test/test-sms-msg-state-change create mode 100644 unit/test-sms-msg-id.c ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[patch 03/20] manpage: explain debugging options to -d
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com Modified HACKING and man page to have more formation on what are the debugging options and how to enable them. --- HACKING | 10 ++ doc/ofonod.8 |5 - 2 files changed, 14 insertions(+), 1 deletions(-) diff --git a/HACKING b/HACKING index ae420aa..e825185 100644 --- a/HACKING +++ b/HACKING @@ -81,3 +81,13 @@ automatically includes this option. For production installations or distribution packaging it is important that the --enable-maintainer-mode option is NOT used. + +Note multiple arguments to -d can be specified, colon, comma or space +separated. The arguments are relative source code filenames for which +debugging output should be enabled; output shell-style globs are +accepted (e.g.: 'plugins/*:src/main.c'). + +Other debugging settings that can be toggled: + + - Environment variable OFONO_AT_DEBUG (set to 1): enable AT commands + debugging diff --git a/doc/ofonod.8 b/doc/ofonod.8 index 474d7fb..7bb908c 100644 --- a/doc/ofonod.8 +++ b/doc/ofonod.8 @@ -18,7 +18,10 @@ is used to manage \fID-Bus\fP permissions for oFono. .SH OPTIONS .TP .B --debug, -d -Enable debug information output. +Enable debug information output. Note multiple arguments to -d can be +specified, colon, comma or space separated. The arguments are relative +source code filenames for which debugging output should be enabled; +output shell-style globs are accepted (e.g.: plugins/*:src/main.c). .TP .B --nodetach, -n Don't run as daemon in background. -- 1.6.6.1 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[patch 09/20] SMS: document handle_sms_status_report()
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
---
src/sms.c |9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/src/sms.c b/src/sms.c
index 49f3e54..b53f880 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -887,6 +887,15 @@ static void handle_deliver(struct ofono_sms *sms, const
struct sms *incoming)
g_slist_free(l);
}
+
+/*
+ * Handle a delivery/status report has been received for an SMS
+ * apparently sent from here.
+ *
+ * We need to find the message in the SMS manager's
+ * waiting-for-acknoledge queue (sms-tx_wfaq) and remove it. As well,
+ * fill out history for it.
+ */
static void handle_sms_status_report(struct ofono_sms *sms,
const struct sms *incoming)
{
--
1.6.6.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[patch 13/20] SMS: encapsulate D-Bus specific data in 'struct sms_msg_dbus_data'
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
This will ease adding later on other fields that are D-Bus specific
and needed to export SMS messages in TX transit over D-Bus.
---
src/sms.c | 21 +++--
1 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/sms.c b/src/sms.c
index 5f5df04..7bde919 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -90,6 +90,17 @@ struct tx_queue_entry {
ofono_destroy_func destroy;
};
+/*
+ * Encapsulate information needed to export an SMS message over D-Bus.
+ *
+ * @dbus_path: path of the object in the D-Bus
+ * @dbus_msg: message this originated at
+ */
+struct sms_msg_dbus_data {
+ char *dbus_path;
+ DBusMessage *dbus_msg;
+};
+
static const char *sms_bearer_to_string(int bearer)
{
switch (bearer) {
@@ -560,7 +571,8 @@ static struct tx_queue_entry *tx_queue_entry_new(GSList
*msg_list)
static void send_message_cb(gboolean ok, void *data)
{
DBusConnection *conn = ofono_dbus_get_connection();
- DBusMessage *msg = data;
+ struct sms_msg_dbus_data *dbus_data = data;
+ DBusMessage *msg = dbus_data-dbus_msg;
DBusMessage *reply;
if (ok)
@@ -573,9 +585,11 @@ static void send_message_cb(gboolean ok, void *data)
static void send_message_destroy(void *data)
{
- DBusMessage *msg = data;
+ struct sms_msg_dbus_data *dbus_data = data;
+ DBusMessage *msg = dbus_data-dbus_msg;
dbus_message_unref(msg);
+ g_free(dbus_data);
}
/*
@@ -590,6 +604,9 @@ static void send_message_destroy(void *data)
* is created by tx_queue_entry_new() and g_queue_push_tail()
* appends that entry to the SMS transmit queue. Then the tx_next()
* function is scheduled to run to process the queue.
+ *
+ * @sms is the main SMS driver struct, @entry and @msg_list represent
+ * the current message being processed.
*/
static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
void *data)
--
1.6.6.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
[patch 15/20] SMS: introduce Wait-for-Status-Report state and infrastructure
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
A WSR (Waiting for Status Report) state definition is added to the
state transtition machine; as well, a queue (ofono_sms-tx_wfaq) where
messages waiting for an status report are queued. When the message's
delivery is acknowledged, the message is removed from the queue and
the message's status machine is updated, which can trigger things such
as D-Bus signals.
---
src/sms.c | 70 +---
1 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/src/sms.c b/src/sms.c
index de6eb7f..1075f33 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -67,6 +67,11 @@ const char *ofono_sms_tx_state_to_string(enum
ofono_sms_tx_state status)
}
+/**
+ * @tx_wsrq: Waiting-for-Status-Report queue; messages in this queue
+ * have been delivered but are waiting to be acknoledged by the
+ * network.
+ */
struct ofono_sms {
int flags;
DBusMessage *pending;
@@ -74,6 +79,7 @@ struct ofono_sms {
struct sms_assembly *assembly;
guint ref;
GQueue *txq;
+ GQueue *tx_wsrq;
gint tx_source;
struct ofono_message_waiting *mw;
unsigned int mw_watch;
@@ -487,10 +493,19 @@ static void __ofono_sms_tx_state_set(struct
tx_queue_entry *entry,
case OFONO_SMS_TX_ST_QUEUED:
ofono_sms_tx_state_check(
file, line, entry, state_old, state_new,
+ 1 OFONO_SMS_TX_ST_WSR
+ | 1 OFONO_SMS_TX_ST_DONE
+ | 1 OFONO_SMS_TX_ST_CANCELING);
+ g_queue_remove(entry-sms_mgr-txq, entry);
+ break;
+ case OFONO_SMS_TX_ST_WSR:
+ ofono_sms_tx_state_check(
+ file, line, entry, state_old, state_new,
1 OFONO_SMS_TX_ST_DONE
| 1 OFONO_SMS_TX_ST_CANCELING
- | 1 OFONO_SMS_TX_ST_FAILED);
- g_queue_remove(entry-sms_mgr-txq, entry);
+ | 1 OFONO_SMS_TX_ST_FAILED
+ | 1 OFONO_SMS_TX_ST_EXPIRED);
+ g_queue_remove(entry-sms_mgr-tx_wsrq, entry);
break;
case OFONO_SMS_TX_ST_CANCELING:
ofono_sms_tx_state_check(
@@ -522,8 +537,8 @@ static void __ofono_sms_tx_state_set(struct tx_queue_entry
*entry,
* Destroy/release the contents of a 'struct tx_queue_entry'
*
* This releases resources allocated *inside* @entry and @entry
- * itself. We blindly remove from the submission queue, in case it is
- * still in any. We could do it with a 'checked' state change to
+ * itself. We blindly remove from both submission queues, in case it
+ * is still in any. We could do it with a 'checked' state change to
* INVALID if we modify that function.
*/
static void tx_queue_entry_destroy_free(gpointer _entry, gpointer unused)
@@ -534,6 +549,7 @@ static void tx_queue_entry_destroy_free(gpointer _entry,
gpointer unused)
entry-destroy(entry-data);
g_free(entry-pdus);
g_queue_remove(entry-sms_mgr-txq, entry);
+ g_queue_remove(entry-sms_mgr-tx_wsrq, entry);
entry-state = __OFONO_SMS_TX_ST_INVALID;
g_free(entry);
}
@@ -586,6 +602,7 @@ static void tx_finished(const struct ofono_error *error,
int mr, void *data)
next_q:
entry = g_queue_peek_head(sms-txq);
+#warning FIXME: cb should not be called on WSR?
if (entry-cb)
entry-cb(ok, entry-data);
@@ -599,12 +616,16 @@ next_q:
__ofono_history_sms_send_status(modem, entry-msg_id,
time(NULL), hs);
}
- if (ok)
+ if (ok entry-flags OFONO_SMS_SUBMIT_FLAG_REQUEST_SR) {
+ ofono_sms_tx_state_set(entry, OFONO_SMS_TX_ST_WSR);
+ g_queue_push_tail(sms-tx_wsrq, entry);
+ } else if (ok !(entry-flags OFONO_SMS_SUBMIT_FLAG_REQUEST_SR)) {
ofono_sms_tx_state_set(entry, OFONO_SMS_TX_ST_DONE);
- else
+ tx_queue_entry_destroy_free(entry, NULL);
+ } else {
ofono_sms_tx_state_set(entry, OFONO_SMS_TX_ST_FAILED);
-
- tx_queue_entry_destroy_free(entry, NULL);
+ tx_queue_entry_destroy_free(entry, NULL);
+ }
if (g_queue_peek_head(sms-txq)) {
DBG(Scheduling next);
@@ -1030,11 +1051,31 @@ static void handle_deliver(struct ofono_sms *sms, const
struct sms *incoming)
/*
+ * This function is a g_queue_foreach() functor called by
+ * handle_sms_status_report() to destroy the message that matches the
+ * reported MSG-ID in the SMS manager's list of messages waiting for
+ * acknowledgement.
+ */
+static void __sms_find_destroy_by_msg_id(gpointer _sms_msg, gpointer _msg_id)
+{
+ unsigned msg_id = (unsigned) _msg_id;
+ struct tx_queue_entry *sms_msg = _sms_msg;
+
+ if (sms_msg-msg_id != msg_id)
+
[patch 14/20] SMS: introduce bare state machine and transitions
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
This adds state to outgoing/in-transit SMS messages. This will be used
later on for persistence / D-Bus, when the SMS life cycle is expanded.
The state is a variable in the 'struct tx_queue_entry' which gets
updated as messages go through the hoops.
---
src/ofono.h | 33
src/sms.c | 121 +--
2 files changed, 150 insertions(+), 4 deletions(-)
diff --git a/src/ofono.h b/src/ofono.h
index 1b72381..51c2743 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -183,6 +183,39 @@ enum ofono_sms_submit_flag {
OFONO_SMS_SUBMIT_FLAG_RETRY = 0x4,
};
+/*
+ * SMS TX message's state
+ *
+ * When a message is queued to be delivered, it will transition
+ * through a set of states.
+ *
+ * Allowed transition table (Allowed, Not-allowed) from left to right:
+ *
+ * UNINITIALIZED CANCELING FAILED
+ * | QUEUED WSR DONE | CANCELLED EXPIRED
+ * UNINITIALIZED -ANNNNNN
+ * QUEUEDN-AAANAN
+ * WSR NN-AANAA
+ * DONE ANN-NNNN
+ * CANCELING NNNN-AAA
+ * CANCELLED ANNNN-NN
+ * FAILEDANNNNN-N
+ * EXPIRED ANNNNNN-
+ */
+enum ofono_sms_tx_state {
+ OFONO_SMS_TX_ST_UNINITIALIZED,
+ OFONO_SMS_TX_ST_QUEUED,
+ OFONO_SMS_TX_ST_WSR,
+ OFONO_SMS_TX_ST_DONE,
+ OFONO_SMS_TX_ST_CANCELING,
+ OFONO_SMS_TX_ST_CANCELLED,
+ OFONO_SMS_TX_ST_FAILED,
+ OFONO_SMS_TX_ST_EXPIRED,
+ __OFONO_SMS_TX_ST_INVALID,
+};
+
+const char *ofono_sms_tx_state_to_string(enum ofono_sms_tx_state);
+
typedef void (*ofono_sms_txq_submit_cb_t)(gboolean ok, void *data);
struct tx_queue_entry * __ofono_sms_txq_submit(struct ofono_sms *sms, GSList
*list,
diff --git a/src/sms.c b/src/sms.c
index 7bde919..de6eb7f 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -50,6 +50,23 @@ static gboolean tx_next(gpointer user_data);
static GSList *g_drivers = NULL;
+const char *ofono_sms_tx_state_to_string(enum ofono_sms_tx_state status)
+{
+ switch (status) {
+ case OFONO_SMS_TX_ST_UNINITIALIZED: return uninitialized;
+ case OFONO_SMS_TX_ST_QUEUED:return queued;
+ case OFONO_SMS_TX_ST_WSR: return wsr;
+ case OFONO_SMS_TX_ST_DONE: return done;
+ case OFONO_SMS_TX_ST_CANCELING: return canceling;
+ case OFONO_SMS_TX_ST_CANCELLED: return cancelled;
+ case OFONO_SMS_TX_ST_FAILED:return failed;
+ case OFONO_SMS_TX_ST_EXPIRED: return expired;
+ default:
+ return invalid;
+ }
+}
+
+
struct ofono_sms {
int flags;
DBusMessage *pending;
@@ -77,11 +94,17 @@ struct pending_pdu {
int pdu_len;
};
+/*
+ * @sms_mgr: SMS manager / driver object
+ * @state: Current state of the (in-transit) SMS
+ */
struct tx_queue_entry {
+ struct ofono_sms *sms_mgr;
struct pending_pdu *pdus;
unsigned char num_pdus;
unsigned char cur_pdu;
struct sms_address receiver;
+ enum ofono_sms_tx_state state;
unsigned int msg_id;
unsigned int retry;
unsigned int flags;
@@ -417,11 +440,91 @@ static DBusMessage *sms_set_property(DBusConnection
*conn, DBusMessage *msg,
}
+/* Check if a state transition is legal */
+static void ofono_sms_tx_state_check(const char *file, unsigned line,
+const struct tx_queue_entry *entry,
+enum ofono_sms_tx_state state_old,
+enum ofono_sms_tx_state state_new,
+unsigned states_allowed_bm)
+{
+ if (((1 state_new) states_allowed_bm) == 0)
+ ofono_warn(%s:%d: SW BUG? Forbidden state change
+ %p %u - %u\n,
+ file, line, entry, state_old, state_new);
+}
+
+
+/*
+ * Set a pending SMS's state
+ *
+ * When leaving state, we do basic cleanup/release of resources
+ * allocated when we entered it.
+ *
+ * This is just syntactic sugar that validates that the transition is
+ * correct and warns out otherwise. The transition table is defined in
+ * the doc block for 'enum ofono_sms_tx_state'.
+ *
+ * In case of inconsistency, we just warn and press forward.
+ */
+#define ofono_sms_tx_state_set(entry, new_state) \
+ __ofono_sms_tx_state_set(entry, new_state, __FILE__, __LINE__)
+
+static void __ofono_sms_tx_state_set(struct tx_queue_entry *entry,
+enum ofono_sms_tx_state state_new,
+const char *file, unsigned line)
+{
+ enum
[patch 18/20] SMS: send D-Bus SMS-MSG::PropertyChanged signals when message changes status
From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com
This hooks sms_msg_stch_dbus_cb() into the SMS state change callback
so that a D-Bus signal will be emitted whenever an SMS Message
transitions state. This allows a client to track, if desired, what is
going on with the SMS Message it cares about.
---
src/sms.c | 36 +++-
test/test-sms-msg-state-change | 24
2 files changed, 59 insertions(+), 1 deletions(-)
create mode 100755 test/test-sms-msg-state-change
diff --git a/src/sms.c b/src/sms.c
index 4569cf3..c0b2e00 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -820,6 +820,40 @@ struct tx_queue_entry *sms_msg_send(
/*
+ * Send a PropertyChange signal when the state changes
+ *
+ * D-Bus typing is ss for variable/value.
+ */
+static
+void sms_msg_stch_dbus_cb(void *_dbus_data,
+ enum ofono_sms_tx_state new_state)
+{
+ struct sms_msg_dbus_data *dbus_data = _dbus_data;
+ DBusConnection *dbus_conn = ofono_dbus_get_connection();
+ DBusMessage *dbus_signal;
+ DBusMessageIter iter;
+ const char *property = TXState;
+ const char *new_state_str;
+
+ /*
+* This will be called in the first transition, when all the
+* D-Bus stuff is not yet in place.
+*/
+ if (dbus_data-dbus_path == NULL)
+ return;
+ dbus_signal = dbus_message_new_signal(
+ dbus_data-dbus_path, SMS_MSG_INTERFACE, PropertyChanged);
+ if (!dbus_signal)
+ return;
+ dbus_message_iter_init_append(dbus_signal, iter);
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, property);
+ new_state_str = ofono_sms_tx_state_to_string(new_state);
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, new_state_str);
+ g_dbus_send_message(dbus_conn, dbus_signal);
+}
+
+
+/*
* D-Bus: Send a SMS text message
*
* @conn: D-Bus connection
@@ -846,7 +880,7 @@ static DBusMessage *dbus_sms_msg_send(DBusConnection *conn,
DBusMessage *msg,
dbus_data = g_new0(struct sms_msg_dbus_data, 1);
sms_msg = sms_msg_send(sms, to, text, SMS_MSG_SEND_HISTORY,
- NULL,
+ sms_msg_stch_dbus_cb,
sms_msg_dbus_destroy,
dbus_data);
if (sms_msg == NULL)
diff --git a/test/test-sms-msg-state-change b/test/test-sms-msg-state-change
new file mode 100755
index 000..18486bf
--- /dev/null
+++ b/test/test-sms-msg-state-change
@@ -0,0 +1,24 @@
+#!/usr/bin/python
+
+import gobject
+
+import dbus
+import dbus.mainloop.glib
+
+def property_changed(name, value, path, interface):
+if interface == org.ofono.SMSMessage:
+print %s: name %s value %s interface %s \
+% (path, name, value, interface)
+
+if __name__ == '__main__':
+print FIXME: this should test the whole state change transition
machine
+ dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+ bus = dbus.SystemBus()
+
+ bus.add_signal_receiver(
+property_changed, bus_name=org.ofono,
+signal_name = PropertyChanged, path_keyword=path,
+interface_keyword=interface)
+ mainloop = gobject.MainLoop()
+ mainloop.run()
--
1.6.6.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] stkutil: convert img to xpm
Hi Kristen, On 07/23/2010 04:39 PM, Kristen Carlson Accardi wrote: On Fri, 23 Jul 2010 16:03:16 -0500 Denis Kenzior denk...@gmail.com wrote: Hi Kristen, On 07/23/2010 03:52 PM, Kristen Carlson Accardi wrote: On Fri, 23 Jul 2010 15:03:59 -0500 Denis Kenzior denk...@gmail.com wrote: This also brings up another point. You're assuming that the caller is appending the CLUT right after the image data and massaging the clut offset appropriately. This is a really bad idea since the caller will have to do some significant pre-processing. You can handle this in one of two ways: 1. Assume the calling logic will read the entire image file before calling this function. In this case, modifying the signature as follows might be a good idea: char *stk_image_to_xpm(const unsigned char *file, unsigned short file_len, enum stk_img_scheme scheme, unsigned short img_offset, unsigned short img_len); 2. Assume the calling logic is clever and will optimize reading of the file, including peeking into the image header to determine the where the CLUT is located and reading it. In this case you can either reuse the signature from 1 above, or come up with something else. Remember, reading from the SIM is extremely slow, so any and all clever optimization tricks are definitely wanted. So, is it likely given normal usage that we'll access an image a single image at a time, or is it more likely that we'll access a bunch of images all at once? It may be better to read an entire image data file (with multiple images) and keep it cached if we are likely to immediately need the other images. In which case I'd be inclined to just pass in the entire data image file and the offset like you have above. If we are only likely to use a single image for any given length of time, then it seems better to have the caller be smart and pass us the clut. The problem is we just don't know, so we have to assume the worst case. Anything that minimizes the number of reads is a good thing (TM). For instance, you might have couple of dozen images from EFimg dispersed among multiple EFiidf files. Each EFiidf file might be 65K in length, but EFimg files might only refer to about 10k from all of them. It is perfectly OK for EFiidf to contain mostly garbage (e.g. for future updates, installation of new SIM Toolkit applications on the SIM + associated image data, etc) So you simply can't assume any sort of packing or efficient storage use. In this case, a clever algorithm that minimizes the number of SIM fetches is needed. Regards, -Denis If that is the case then I propose we assume a smart algorithm fetched our data for us and change the signature to something like this: unsigned char *stk_image_to_xpm(const unsigned char *image_body, enum stk_image_scheme scheme, unsigned int height, unsigned int width, unsigned int ncolors, const unsigned char *clut); We would assume that the caller has done all the sanity checking on the file and handed us a clut of appropriate size as well. You sure you want unsigned char * as the return and not char *? Also, you might want to change unsigned ints to unsigned chars. The image size and CLUT size can never be more than 256. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 01/20] bug.h: Add BUILD_BUG_ON() and friends for compile-time assert checking
Hi Inaky, Second, please actually check that your code passes make distcheck: CCLD unit/test-idmap CC unit/test-sms.o CC src/smsutil.o ../src/smsutil.c:40:17: error: bug.h: No such file or directory make[2]: *** [src/smsutil.o] Error 1 make[1]: *** [all] Error 2 make: *** [distcheck] Error 1 another FAIL. However, I am hitting another one in missing ofono/types.h, even after having pulled the tip as of today. Is that one known? Anyway, fixing this one. Make distcheck passes fine on $HEAD for me. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] stkutil: convert img to xpm
On Fri, 23 Jul 2010 16:46:51 -0500 Denis Kenzior denk...@gmail.com wrote: Hi Kristen, On 07/23/2010 04:39 PM, Kristen Carlson Accardi wrote: On Fri, 23 Jul 2010 16:03:16 -0500 Denis Kenzior denk...@gmail.com wrote: Hi Kristen, On 07/23/2010 03:52 PM, Kristen Carlson Accardi wrote: On Fri, 23 Jul 2010 15:03:59 -0500 Denis Kenzior denk...@gmail.com wrote: This also brings up another point. You're assuming that the caller is appending the CLUT right after the image data and massaging the clut offset appropriately. This is a really bad idea since the caller will have to do some significant pre-processing. You can handle this in one of two ways: 1. Assume the calling logic will read the entire image file before calling this function. In this case, modifying the signature as follows might be a good idea: char *stk_image_to_xpm(const unsigned char *file, unsigned short file_len, enum stk_img_scheme scheme, unsigned short img_offset, unsigned short img_len); 2. Assume the calling logic is clever and will optimize reading of the file, including peeking into the image header to determine the where the CLUT is located and reading it. In this case you can either reuse the signature from 1 above, or come up with something else. Remember, reading from the SIM is extremely slow, so any and all clever optimization tricks are definitely wanted. So, is it likely given normal usage that we'll access an image a single image at a time, or is it more likely that we'll access a bunch of images all at once? It may be better to read an entire image data file (with multiple images) and keep it cached if we are likely to immediately need the other images. In which case I'd be inclined to just pass in the entire data image file and the offset like you have above. If we are only likely to use a single image for any given length of time, then it seems better to have the caller be smart and pass us the clut. The problem is we just don't know, so we have to assume the worst case. Anything that minimizes the number of reads is a good thing (TM). For instance, you might have couple of dozen images from EFimg dispersed among multiple EFiidf files. Each EFiidf file might be 65K in length, but EFimg files might only refer to about 10k from all of them. It is perfectly OK for EFiidf to contain mostly garbage (e.g. for future updates, installation of new SIM Toolkit applications on the SIM + associated image data, etc) So you simply can't assume any sort of packing or efficient storage use. In this case, a clever algorithm that minimizes the number of SIM fetches is needed. Regards, -Denis If that is the case then I propose we assume a smart algorithm fetched our data for us and change the signature to something like this: unsigned char *stk_image_to_xpm(const unsigned char *image_body, enum stk_image_scheme scheme, unsigned int height, unsigned int width, unsigned int ncolors, const unsigned char *clut); We would assume that the caller has done all the sanity checking on the file and handed us a clut of appropriate size as well. You sure you want unsigned char * as the return and not char *? Oops - no, I typed that wrong. Also, you might want to change unsigned ints to unsigned chars. The image size and CLUT size can never be more than 256. The image size can be greater than 256, but I think you mean the value of height, width, and ncolors can never be greater than 256, which is true. ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/2] stkutil: convert img to xpm
Hi Kristen, Also, you might want to change unsigned ints to unsigned chars. The image size and CLUT size can never be more than 256. The image size can be greater than 256, but I think you mean the value of height, width, and ncolors can never be greater than 256, which is true. Sorry, yes that is what I meant. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 03/20] manpage: explain debugging options to -d
Hi Inaky, On 07/23/2010 03:59 PM, Inaky Perez-Gonzalez wrote: From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com Modified HACKING and man page to have more formation on what are the debugging options and how to enable them. --- HACKING | 10 ++ doc/ofonod.8 |5 - 2 files changed, 14 insertions(+), 1 deletions(-) This one has been applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 06/20] _assembly_encode_address: export and rename
Hi Inaky, On 07/23/2010 03:59 PM, Inaky Perez-Gonzalez wrote: From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com The new name better reflects the function's purpose. We need to export it, as for generating unique message naming (for persistence and D-Bus object naming), we'll be using the address. --- src/smsutil.c |7 +++ src/smsutil.h |2 ++ 2 files changed, 5 insertions(+), 4 deletions(-) Applied with a very small tweak to the commit header. Thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 10/20] sms_text_prepare: document @use_delivery_reports
Hi Inaky, On 07/23/2010 03:59 PM, Inaky Perez-Gonzalez wrote: From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com --- src/smsutil.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) This one has been applied with a minor tweak to the commit message. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 11/20] SMS: rename create_tx_queue_entry() to tx_queue_entry_new()
Hi Inaky, On 07/23/2010 03:59 PM, Inaky Perez-Gonzalez wrote: From: Inaky Perez-Gonzalez inaky.perez-gonza...@intel.com This is for symmetry with tx_queue_entry_free() --- src/sms.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) Applied with a minor tweak to the commit header. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [patch 12/20] struct tx_queue_entry: add a destructor
Hi Inaky,
tx_queue_entry * here. Casting the destructor in g_foo_foreach is
acceptable.
Heh, I'd rather have it like this: Repeat after me in a 1000-loop: casts
are evil and should be avoided for they muck the code by hiding warnings
when you change things and miss updating all the sites ...
I fully agree, but the extra silly NULL argument makes this worse than
the cast. And to be clear, g_foo_foreach is about the only place we
explicitly allow casts. If you feel you need to be pedantic, then write
a GFunc specifically to do the foreach bit.
e.g. tx_queue_entry_free(struct tx_queue_entry *);
txq_foreach_free(gpointer foo, gpointer user)
{
struct tx_queue_entry *bar = foo;
tx_queue_entry_free(bar);
}
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
