12:46 a.m.
Hello,
Please find the changes in order to correct call forwarding states.
Changes from v2:
- Re-run conditional queries on cfu removal and mark cached.
- Handle the caching properly on supplementary services path
unconditional/all/all conditional cfs modifications.
Regards,
Oleg
Oleg Zhurakivskyy (12):
call-forwarding: Remove cf_list_clear()
call-forwarding: Inline get_query_next_cf_cond()
call-forwarding: Refactor cf_condition_find_with_cls() slightly
call-forwarding: Refactor cf_find_unconditional()
call-forwarding: Minor cleanup of set_query_cf_callback
call-forwarding: Don't run conditional queries if cfu is active
call-forwarding: Re-run conditional queries on cfu removal
call-forwarding: Toggle the cached flag on CFU changes
call-forwarding: Cache cfs on CFU removal
call-forwarding: Re-run ss path cfs queries on cfu changes
call-forwarding: Cache cfs on all/all conditional removal
TODO: Remove completed call forwarding state task
TODO | 17 ----
src/call-forwarding.c | 237 +++++++++++++++++++++----------------------------
2 files changed, 100 insertions(+), 154 deletions(-)
--
1.7.5.4
12:46 a.m.
New subject: [PATCHv3 01/12] call-forwarding: Remove cf_list_clear()
Use g_slist_free_full() instead.
---
src/call-forwarding.c | 14 ++------------
1 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 864be1b..0857362 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -183,22 +183,12 @@ static GSList *cf_cond_list_create(int total,
return l;
}
-static inline void cf_list_clear(GSList *cf_list)
-{
- GSList *l;
-
- for (l = cf_list; l; l = l->next)
- g_free(l->data);
-
- g_slist_free(cf_list);
-}
-
static inline void cf_clear_all(struct ofono_call_forwarding *cf)
{
int i;
for (i = 0; i < 4; i++) {
- cf_list_clear(cf->cf_conditions[i]);
+ g_slist_free_full(cf->cf_conditions[i], g_free);
cf->cf_conditions[i] = NULL;
}
}
@@ -422,7 +412,7 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
&timeout);
}
- cf_list_clear(old);
+ g_slist_free_full(old, g_free);
cf->cf_conditions[type] = list;
if (update_sim == TRUE)
--
1.7.5.4
6:03 a.m.
New subject: [PATCHv3 01/12] call-forwarding: Remove cf_list_clear()
Hi Oleg,
On 03/13/2012 08:46 AM, Oleg Zhurakivskyy wrote:
Use g_slist_free_full() instead.
---
src/call-forwarding.c | 14 ++------------
1 files changed, 2 insertions(+), 12 deletions(-)
Patch has been applied, thanks.
Regards,
-Denis
12:46 a.m.
New subject: [PATCHv3 02/12] call-forwarding: Inline get_query_next_cf_cond()
---
src/call-forwarding.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 0857362..8b6659a 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -595,7 +595,7 @@ static void get_query_cf_callback(const struct ofono_error *error, int
total,
get_query_next_cf_cond(cf);
}
-static void get_query_next_cf_cond(struct ofono_call_forwarding *cf)
+static inline void get_query_next_cf_cond(struct ofono_call_forwarding *cf)
{
cf->driver->query(cf, cf->query_next, BEARER_CLASS_DEFAULT,
get_query_cf_callback, cf);
--
1.7.5.4
12:46 a.m.
New subject: [PATCHv3 03/12] call-forwarding: Refactor cf_condition_find_with_cls() slightly
Mainly to simplify the usage.
---
src/call-forwarding.c | 98 +++++++++++++-----------------------------------
1 files changed, 27 insertions(+), 71 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 8b6659a..5a2ab28 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -42,6 +42,8 @@
/* According to 27.007 Spec */
#define DEFAULT_NO_REPLY_TIMEOUT 20
+#define CFC(_x) ((struct ofono_call_forwarding_condition *)(_x))
+
#define is_cfu_enabled(_cf) \
({ \
cf_find_unconditional(_cf) ? TRUE : FALSE; \
@@ -86,63 +88,36 @@ static void get_query_next_cf_cond(struct ofono_call_forwarding *cf);
static void set_query_next_cf_cond(struct ofono_call_forwarding *cf);
static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf);
-static gint cf_condition_compare(gconstpointer a, gconstpointer b)
+static gint cf_compare(gconstpointer a, gconstpointer b)
{
- const struct ofono_call_forwarding_condition *ca = a;
- const struct ofono_call_forwarding_condition *cb = b;
-
- if (ca->cls < cb->cls)
- return -1;
-
- if (ca->cls > cb->cls)
- return 1;
-
- return 0;
+ return CFC(a)->cls - CFC(b)->cls;
}
-static gint cf_condition_find_with_cls(gconstpointer a, gconstpointer b)
+static struct ofono_call_forwarding_condition *cf_find(GSList *l, int cls)
{
- const struct ofono_call_forwarding_condition *c = a;
- int cls = GPOINTER_TO_INT(b);
-
- if (c->cls < cls)
- return -1;
-
- if (c->cls > cls)
- return 1;
+ for (; l; l = l->next)
+ if (cls == CFC(l->data)->cls)
+ return CFC(l->data);
- return 0;
+ return NULL;
}
-static int cf_find_timeout(GSList *cf_list, int cls)
+static int cf_find_timeout(GSList *l, int cls)
{
- GSList *l;
- struct ofono_call_forwarding_condition *c;
-
- l = g_slist_find_custom(cf_list, GINT_TO_POINTER(cls),
- cf_condition_find_with_cls);
-
- if (l == NULL)
- return DEFAULT_NO_REPLY_TIMEOUT;
-
- c = l->data;
+ struct ofono_call_forwarding_condition *c = cf_find(l, cls);
- return c->time;
+ return c ? c->time : DEFAULT_NO_REPLY_TIMEOUT;
}
-static void cf_cond_list_print(GSList *list)
+static void cf_cond_list_print(GSList *l)
{
- GSList *l;
- struct ofono_call_forwarding_condition *cond;
-
- for (l = list; l; l = l->next) {
- cond = l->data;
+ struct ofono_call_forwarding_condition *c;
+ for (; l && (c = l->data); l = l->next)
DBG("CF Condition status: %d, class: %d, number: %s,"
" number_type: %d, time: %d",
- cond->status, cond->cls, cond->phone_number.number,
- cond->phone_number.type, cond->time);
- }
+ c->status, c->cls, c->phone_number.number,
+ c->phone_number.type, c->time);
}
static GSList *cf_cond_list_create(int total,
@@ -175,8 +150,7 @@ static GSList *cf_cond_list_create(int total,
sizeof(struct ofono_call_forwarding_condition));
cond->cls = j;
- l = g_slist_insert_sorted(l, cond,
- cf_condition_compare);
+ l = g_slist_insert_sorted(l, cond, cf_compare);
}
}
@@ -330,12 +304,8 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
if (type == CALL_FORWARDING_TYPE_NO_REPLY)
snprintf(tattr, sizeof(tattr), "%sTimeout", attr);
- o = g_slist_find_custom(old, GINT_TO_POINTER(lc->cls),
- cf_condition_find_with_cls);
-
- if (o) { /* On the old list, must be active */
- oc = o->data;
-
+ oc = cf_find(old, lc->cls);
+ if (oc) { /* On the old list, must be active */
if (oc->phone_number.type != lc->phone_number.type ||
strcmp(oc->phone_number.number,
lc->phone_number.number)) {
@@ -356,8 +326,8 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
&timeout);
/* Remove from the old list */
- g_free(o->data);
- old = g_slist_remove(old, o->data);
+ old = g_slist_remove(old, oc);
+ g_free(oc);
} else {
number = phone_number_to_string(&lc->phone_number);
@@ -435,21 +405,12 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
if (i == CALL_FORWARDING_TYPE_UNCONDITIONAL)
continue;
- l = g_slist_find_custom(cf->cf_conditions[i],
- GINT_TO_POINTER(BEARER_CLASS_VOICE),
- cf_condition_find_with_cls);
-
- if (l == NULL)
+ lc = cf_find(cf->cf_conditions[i], BEARER_CLASS_VOICE);
+ if (lc == NULL)
continue;
- if (new_cfu)
- number = "";
- else {
- lc = l->data;
-
- number = phone_number_to_string(
+ number = new_cfu ? "" : phone_number_to_string(
&lc->phone_number);
- }
ofono_dbus_signal_property_changed(conn, path,
OFONO_CALL_FORWARDING_INTERFACE,
@@ -796,7 +757,6 @@ static DBusMessage *cf_set_property(DBusConnection *conn, DBusMessage
*msg,
if (cf_condition_timeout_property(property, &cls)) {
dbus_uint16_t timeout;
- GSList *l;
struct ofono_call_forwarding_condition *c;
type = CALL_FORWARDING_TYPE_NO_REPLY;
@@ -809,15 +769,11 @@ static DBusMessage *cf_set_property(DBusConnection *conn,
DBusMessage *msg,
if (timeout < 1 || timeout > 30)
return __ofono_error_invalid_format(msg);
- l = g_slist_find_custom(cf->cf_conditions[type],
- GINT_TO_POINTER(cls),
- cf_condition_find_with_cls);
- if (l == NULL)
+ c = cf_find(cf->cf_conditions[type], cls);
+ if (c == NULL)
return __ofono_error_failed(msg);
- c = l->data;
-
return set_property_request(cf, msg, type, cls,
&c->phone_number, timeout);
} else if (cf_condition_enabled_property(cf, property, &type, &cls)) {
--
1.7.5.4
10:50 a.m.
New subject: [PATCHv3 03/12] call-forwarding: Refactor cf_condition_find_with_cls() slightly
Hi Oleg,
On 03/13/2012 08:46 AM, Oleg Zhurakivskyy wrote:
Mainly to simplify the usage.
---
src/call-forwarding.c | 98 +++++++++++++-----------------------------------
1 files changed, 27 insertions(+), 71 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 8b6659a..5a2ab28 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -42,6 +42,8 @@
/* According to 27.007 Spec */
#define DEFAULT_NO_REPLY_TIMEOUT 20
+#define CFC(_x) ((struct ofono_call_forwarding_condition *)(_x))
+
I'm okay with such helpers, but...
#define is_cfu_enabled(_cf) \
({ \
cf_find_unconditional(_cf) ? TRUE : FALSE; \
@@ -86,63 +88,36 @@ static void get_query_next_cf_cond(struct ofono_call_forwarding
*cf);
static void set_query_next_cf_cond(struct ofono_call_forwarding *cf);
static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf);
-static gint cf_condition_compare(gconstpointer a, gconstpointer b)
+static gint cf_compare(gconstpointer a, gconstpointer b)
{
- const struct ofono_call_forwarding_condition *ca = a;
- const struct ofono_call_forwarding_condition *cb = b;
-
- if (ca->cls < cb->cls)
- return -1;
-
- if (ca->cls > cb->cls)
- return 1;
-
- return 0;
+ return CFC(a)->cls - CFC(b)->cls;
You're breaking const-correctness here.
Also, why do you rename the function? I do think we should have
'condition' or at least 'cond' in the name to make it clear what it
does. My vote is for cf_cond_compare...
}
-static gint cf_condition_find_with_cls(gconstpointer a, gconstpointer b)
+static struct ofono_call_forwarding_condition *cf_find(GSList *l, int cls)
Same as above, at least name it cf_cond_find. Also, I'd prefer changes
to this function to be in a separate patch.
{
- const struct ofono_call_forwarding_condition *c = a;
- int cls = GPOINTER_TO_INT(b);
-
- if (c->cls < cls)
- return -1;
-
- if (c->cls > cls)
- return 1;
+ for (; l; l = l->next)
+ if (cls == CFC(l->data)->cls)
+ return CFC(l->data);
- return 0;
+ return NULL;
}
-static int cf_find_timeout(GSList *cf_list, int cls)
+static int cf_find_timeout(GSList *l, int cls)
Same here, lets name this cf_cond_find_timeout
{
- GSList *l;
- struct ofono_call_forwarding_condition *c;
-
- l = g_slist_find_custom(cf_list, GINT_TO_POINTER(cls),
- cf_condition_find_with_cls);
-
- if (l == NULL)
- return DEFAULT_NO_REPLY_TIMEOUT;
-
- c = l->data;
+ struct ofono_call_forwarding_condition *c = cf_find(l, cls);
- return c->time;
+ return c ? c->time : DEFAULT_NO_REPLY_TIMEOUT;
}
-static void cf_cond_list_print(GSList *list)
+static void cf_cond_list_print(GSList *l)
{
- GSList *l;
- struct ofono_call_forwarding_condition *cond;
-
- for (l = list; l; l = l->next) {
- cond = l->data;
+ struct ofono_call_forwarding_condition *c;
+ for (; l && (c = l->data); l = l->next)
After going back and forth on this one, I'm actually against this style.
Mainly it is too hard to notice the assignment, inside the continuation
check expression. I'd really prefer that we assign c inside the for
loop block. Also, l->data should never be NULL, so in this case I'd
rather crash and find out early than inadvertently covering it up.
DBG("CF Condition status: %d, class: %d, number: %s,"
" number_type: %d, time: %d",
- cond->status, cond->cls, cond->phone_number.number,
- cond->phone_number.type, cond->time);
- }
+ c->status, c->cls, c->phone_number.number,
+ c->phone_number.type, c->time);
}
static GSList *cf_cond_list_create(int total,
@@ -175,8 +150,7 @@ static GSList *cf_cond_list_create(int total,
sizeof(struct ofono_call_forwarding_condition));
cond->cls = j;
- l = g_slist_insert_sorted(l, cond,
- cf_condition_compare);
+ l = g_slist_insert_sorted(l, cond, cf_compare);
}
}
@@ -330,12 +304,8 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
if (type == CALL_FORWARDING_TYPE_NO_REPLY)
snprintf(tattr, sizeof(tattr), "%sTimeout", attr);
- o = g_slist_find_custom(old, GINT_TO_POINTER(lc->cls),
- cf_condition_find_with_cls);
-
- if (o) { /* On the old list, must be active */
- oc = o->data;
-
+ oc = cf_find(old, lc->cls);
+ if (oc) { /* On the old list, must be active */
if (oc->phone_number.type != lc->phone_number.type ||
strcmp(oc->phone_number.number,
lc->phone_number.number)) {
@@ -356,8 +326,8 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
&timeout);
/* Remove from the old list */
- g_free(o->data);
- old = g_slist_remove(old, o->data);
+ old = g_slist_remove(old, oc);
+ g_free(oc);
} else {
number = phone_number_to_string(&lc->phone_number);
@@ -435,21 +405,12 @@ static void set_new_cond_list(struct ofono_call_forwarding *cf,
if (i == CALL_FORWARDING_TYPE_UNCONDITIONAL)
continue;
- l = g_slist_find_custom(cf->cf_conditions[i],
- GINT_TO_POINTER(BEARER_CLASS_VOICE),
- cf_condition_find_with_cls);
-
- if (l == NULL)
+ lc = cf_find(cf->cf_conditions[i], BEARER_CLASS_VOICE);
+ if (lc == NULL)
continue;
- if (new_cfu)
- number = "";
- else {
- lc = l->data;
-
- number = phone_number_to_string(
+ number = new_cfu ? "" : phone_number_to_string(
&lc->phone_number);
This part really belongs in a separate patch.
- }
ofono_dbus_signal_property_changed(conn, path,
OFONO_CALL_FORWARDING_INTERFACE,
@@ -796,7 +757,6 @@ static DBusMessage *cf_set_property(DBusConnection *conn, DBusMessage
*msg,
if (cf_condition_timeout_property(property, &cls)) {
dbus_uint16_t timeout;
- GSList *l;
struct ofono_call_forwarding_condition *c;
type = CALL_FORWARDING_TYPE_NO_REPLY;
@@ -809,15 +769,11 @@ static DBusMessage *cf_set_property(DBusConnection *conn,
DBusMessage *msg,
if (timeout < 1 || timeout > 30)
return __ofono_error_invalid_format(msg);
- l = g_slist_find_custom(cf->cf_conditions[type],
- GINT_TO_POINTER(cls),
- cf_condition_find_with_cls);
- if (l == NULL)
+ c = cf_find(cf->cf_conditions[type], cls);
+ if (c == NULL)
return __ofono_error_failed(msg);
- c = l->data;
-
return set_property_request(cf, msg, type, cls,
&c->phone_number, timeout);
} else if (cf_condition_enabled_property(cf, property, &type, &cls)) {
Regards,
-Denis
10:45 p.m.
New subject: [PATCHv3 03/12] call-forwarding: Refactor cf_condition_find_with_cls() slightly
Hello Denis,
On 03/21/2012 01:50 AM, Denis Kenzior wrote:
> + return CFC(a)->cls - CFC(b)->cls;
You're breaking const-correctness here.
Thanks for pointing this. The intention was to save a few instructions here, but
just missed that.
Also, why do you rename the function? I do think we should have
'condition' or at least 'cond' in the name to make it clear what it
does. My vote is for cf_cond_compare...
Thanks, I agree, cf_cond_compare() would be more consistent and clear.
> +static struct ofono_call_forwarding_condition *cf_find(GSList
*l, int cls)
Same as above, at least name it cf_cond_find. Also, I'd prefer changes
to this function to be in a separate patch.
OK.
> -static int cf_find_timeout(GSList *cf_list, int cls)
> +static int cf_find_timeout(GSList *l, int cls)
Same here, lets name this cf_cond_find_timeout
OK.
> -static void cf_cond_list_print(GSList *list)
> +static void cf_cond_list_print(GSList *l)
> {
> - GSList *l;
> - struct ofono_call_forwarding_condition *cond;
> -
> - for (l = list; l; l = l->next) {
> - cond = l->data;
> + struct ofono_call_forwarding_condition *c;
>
> + for (; l&& (c = l->data); l = l->next)
After going back and forth on this one, I'm actually against this style.
Mainly it is too hard to notice the assignment, inside the continuation
check expression. I'd really prefer that we assign c inside the for
loop block. Also, l->data should never be NULL, so in this case I'd
rather crash and find out early than inadvertently covering it up.
OK, thanks. I agree on both points here.
> + number = new_cfu ? "" : phone_number_to_string(
> &lc->phone_number);
This part really belongs in a separate patch.
OK.
Thanks for the help, I will correct all these.
Regards,
Oleg
--
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
12:46 a.m.
New subject: [PATCHv3 04/12] call-forwarding: Refactor cf_find_unconditional()
Re-use cf_find() instead.
---
src/call-forwarding.c | 28 ++++++----------------------
1 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 5a2ab28..b1a1a72 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -44,6 +44,12 @@
#define CFC(_x) ((struct ofono_call_forwarding_condition *)(_x))
+#define cf_find_unconditional(_cf) \
+({ \
+ cf_find((_cf)->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL],\
+ BEARER_CLASS_VOICE); \
+})
+
#define is_cfu_enabled(_cf) \
({ \
cf_find_unconditional(_cf) ? TRUE : FALSE; \
@@ -188,28 +194,6 @@ static void sim_cphs_cff_update_cb(int ok, void *data)
ofono_info("Failed to update EFcphs-cff");
}
-static struct ofono_call_forwarding_condition *cf_find_unconditional(
- struct ofono_call_forwarding *cf)
-{
- GSList *l = cf->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL];
- struct ofono_call_forwarding_condition *cond;
-
- /*
- * For now we only support Voice, although Fax & all Data
- * basic services are applicable as well.
- */
- for (; l; l = l->next) {
- cond = l->data;
-
- if (cond->cls > BEARER_CLASS_VOICE)
- continue;
-
- return cond;
- }
-
- return NULL;
-}
-
static void sim_set_cf_indicator(struct ofono_call_forwarding *cf)
{
struct ofono_call_forwarding_condition *cfu_voice =
--
1.7.5.4
10:51 a.m.
New subject: [PATCHv3 04/12] call-forwarding: Refactor cf_find_unconditional()
On 03/13/2012 08:46 AM, Oleg Zhurakivskyy wrote:
Re-use cf_find() instead.
---
src/call-forwarding.c | 28 ++++++----------------------
1 files changed, 6 insertions(+), 22 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 5a2ab28..b1a1a72 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -44,6 +44,12 @@
#define CFC(_x) ((struct ofono_call_forwarding_condition *)(_x))
+#define cf_find_unconditional(_cf) \
+({ \
+ cf_find((_cf)->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL],\
+ BEARER_CLASS_VOICE); \
+})
+
Actually I'd prefer if we made this an inline function rather than a
macro. A bit easier to read that way.
Regards,
-Denis
10:46 p.m.
New subject: [PATCHv3 04/12] call-forwarding: Refactor cf_find_unconditional()
Hello Denis,
On 03/21/2012 01:51 AM, Denis Kenzior wrote:
> +#define cf_find_unconditional(_cf) \
> +({ \
> + cf_find((_cf)->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL],\
> + BEARER_CLASS_VOICE); \
> +})
> +
Actually I'd prefer if we made this an inline function rather than a
macro. A bit easier to read that way.
Sure, strongly agree. I was going to do this myself, it just didn't make it into
these patches.
Regards,
Oleg
--
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
12:46 a.m.
New subject: [PATCHv3 05/12] call-forwarding: Minor cleanup of set_query_cf_callback
---
src/call-forwarding.c | 26 +++++++++++---------------
1 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index b1a1a72..7e5edcb 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -631,32 +631,28 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
void *data)
{
struct ofono_call_forwarding *cf = data;
- GSList *l;
- DBusMessage *reply;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
ofono_error("Setting succeeded, but query failed");
cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
- reply = __ofono_error_failed(cf->pending);
- __ofono_dbus_pending_reply(&cf->pending, reply);
+ __ofono_dbus_pending_reply(&cf->pending,
+ __ofono_error_failed(cf->pending));
return;
}
- if (cf->query_next == cf->query_end) {
- reply = dbus_message_new_method_return(cf->pending);
- __ofono_dbus_pending_reply(&cf->pending, reply);
- }
-
- l = cf_cond_list_create(total, list);
- set_new_cond_list(cf, cf->query_next, l);
+ set_new_cond_list(cf, cf->query_next, cf_cond_list_create(total, list));
DBG("%s conditions:", cf_type_lut[cf->query_next]);
- cf_cond_list_print(l);
+ cf_cond_list_print(cf->cf_conditions[cf->query_next]);
- if (cf->query_next != cf->query_end) {
- cf->query_next++;
- set_query_next_cf_cond(cf);
+ if (cf->query_next == cf->query_end) {
+ __ofono_dbus_pending_reply(&cf->pending,
+ dbus_message_new_method_return(cf->pending));
+ return;
}
+
+ cf->query_next++;
+ set_query_next_cf_cond(cf);
}
static void set_query_next_cf_cond(struct ofono_call_forwarding *cf)
--
1.7.5.4
10:54 a.m.
New subject: [PATCHv3 05/12] call-forwarding: Minor cleanup of set_query_cf_callback
Hi Oleg,
On 03/13/2012 08:46 AM, Oleg Zhurakivskyy wrote:
---
src/call-forwarding.c | 26 +++++++++++---------------
1 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index b1a1a72..7e5edcb 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -631,32 +631,28 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
void *data)
{
struct ofono_call_forwarding *cf = data;
- GSList *l;
- DBusMessage *reply;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
ofono_error("Setting succeeded, but query failed");
cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
- reply = __ofono_error_failed(cf->pending);
- __ofono_dbus_pending_reply(&cf->pending, reply);
+ __ofono_dbus_pending_reply(&cf->pending,
+ __ofono_error_failed(cf->pending));
This looks fine...
return;
}
- if (cf->query_next == cf->query_end) {
- reply = dbus_message_new_method_return(cf->pending);
- __ofono_dbus_pending_reply(&cf->pending, reply);
- }
-
Actually this part is here on purpose, we want to generate a reply
before we generate a signal for PropertyChanged.
- l = cf_cond_list_create(total, list);
- set_new_cond_list(cf, cf->query_next, l);
+ set_new_cond_list(cf, cf->query_next, cf_cond_list_create(total, list));
DBG("%s conditions:", cf_type_lut[cf->query_next]);
- cf_cond_list_print(l);
+ cf_cond_list_print(cf->cf_conditions[cf->query_next]);
- if (cf->query_next != cf->query_end) {
- cf->query_next++;
- set_query_next_cf_cond(cf);
+ if (cf->query_next == cf->query_end) {
+ __ofono_dbus_pending_reply(&cf->pending,
+ dbus_message_new_method_return(cf->pending));
+ return;
}
+
+ cf->query_next++;
+ set_query_next_cf_cond(cf);
}
static void set_query_next_cf_cond(struct ofono_call_forwarding *cf)
Regards,
-Denis
10:47 p.m.
New subject: [PATCHv3 05/12] call-forwarding: Minor cleanup of set_query_cf_callback
Hello Denis,
On 03/21/2012 01:54 AM, Denis Kenzior wrote:
> - if (cf->query_next == cf->query_end) {
> - reply = dbus_message_new_method_return(cf->pending);
> - __ofono_dbus_pending_reply(&cf->pending, reply);
> - }
> -
Actually this part is here on purpose, we want to generate a reply
before we generate a signal for PropertyChanged.
Thanks for the help here, it really missed my attention.
Regards,
Oleg
--
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
12:46 a.m.
New subject: [PATCHv3 06/12] call-forwarding: Don't run conditional queries if cfu is active
---
src/call-forwarding.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 7e5edcb..ad5050b 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -525,19 +525,18 @@ static void get_query_cf_callback(const struct ofono_error *error,
int total,
DBG("%s conditions:", cf_type_lut[cf->query_next]);
cf_cond_list_print(l);
-
- if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE)
- cf->flags |= CALL_FORWARDING_FLAG_CACHED;
}
- if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE) {
- DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
- __ofono_dbus_pending_reply(&cf->pending, reply);
+ if (cf->query_next++ < cf->query_end) {
+ get_query_next_cf_cond(cf);
return;
}
- cf->query_next++;
- get_query_next_cf_cond(cf);
+ if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
+ cf->flags |= CALL_FORWARDING_FLAG_CACHED;
+
+ __ofono_dbus_pending_reply(&cf->pending,
+ cf_get_properties_reply(cf->pending, cf));
}
static inline void get_query_next_cf_cond(struct ofono_call_forwarding *cf)
@@ -565,7 +564,9 @@ static DBusMessage *cf_get_properties(DBusConnection *conn,
DBusMessage *msg,
cf->pending = dbus_message_ref(msg);
cf->query_next = 0;
-
+ cf->query_end = is_cfu_enabled(cf) ?
+ CALL_FORWARDING_TYPE_UNCONDITIONAL :
+ CALL_FORWARDING_TYPE_NOT_REACHABLE;
get_query_next_cf_cond(cf);
return NULL;
--
1.7.5.4
11:09 a.m.
New subject: [PATCHv3 06/12] call-forwarding: Don't run conditional queries if cfu is active
Hi Oleg,
On 03/13/2012 08:46 AM, Oleg Zhurakivskyy wrote:
---
src/call-forwarding.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 7e5edcb..ad5050b 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -525,19 +525,18 @@ static void get_query_cf_callback(const struct ofono_error *error,
int total,
DBG("%s conditions:", cf_type_lut[cf->query_next]);
cf_cond_list_print(l);
-
- if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE)
- cf->flags |= CALL_FORWARDING_FLAG_CACHED;
}
- if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE) {
- DBusMessage *reply = cf_get_properties_reply(cf->pending, cf);
- __ofono_dbus_pending_reply(&cf->pending, reply);
+ if (cf->query_next++ < cf->query_end) {
+ get_query_next_cf_cond(cf);
return;
}
- cf->query_next++;
- get_query_next_cf_cond(cf);
+ if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
+ cf->flags |= CALL_FORWARDING_FLAG_CACHED;
+
+ __ofono_dbus_pending_reply(&cf->pending,
+ cf_get_properties_reply(cf->pending, cf));
}
static inline void get_query_next_cf_cond(struct ofono_call_forwarding *cf)
@@ -565,7 +564,9 @@ static DBusMessage *cf_get_properties(DBusConnection *conn,
DBusMessage *msg,
cf->pending = dbus_message_ref(msg);
cf->query_next = 0;
-
+ cf->query_end = is_cfu_enabled(cf) ?
+ CALL_FORWARDING_TYPE_UNCONDITIONAL :
+ CALL_FORWARDING_TYPE_NOT_REACHABLE;
get_query_next_cf_cond(cf);
return NULL;
What if we're running GetProperties for the first time and CFU is active
at the network, but for some reason (e.g. driver doesn't implement it)
the SIM call forwarding rules are absent. Won't we query conditional
CFs needlessly here?
Regards,
-Denis
10:49 p.m.
New subject: [PATCHv3 06/12] call-forwarding: Don't run conditional queries if cfu is active
Hello Denis,
On 03/21/2012 02:09 AM, Denis Kenzior wrote:
> static inline void get_query_next_cf_cond(struct
ofono_call_forwarding *cf)
> @@ -565,7 +564,9 @@ static DBusMessage *cf_get_properties(DBusConnection *conn,
DBusMessage *msg,
>
> cf->pending = dbus_message_ref(msg);
> cf->query_next = 0;
> -
> + cf->query_end = is_cfu_enabled(cf) ?
> + CALL_FORWARDING_TYPE_UNCONDITIONAL :
> + CALL_FORWARDING_TYPE_NOT_REACHABLE;
> get_query_next_cf_cond(cf);
>
> return NULL;
What if we're running GetProperties for the first time and CFU is active
at the network, but for some reason (e.g. driver doesn't implement it)
the SIM call forwarding rules are absent. Won't we query conditional
CFs needlessly here?
Good point and thanks for pointing this. I will correct so, that once the CFU is
discovered and active, the conditional queries will be skipped.
Regards,
Oleg
--
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
12:46 a.m.
New subject: [PATCHv3 07/12] call-forwarding: Re-run conditional queries on cfu removal
---
src/call-forwarding.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index ad5050b..edd1891 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -691,7 +691,15 @@ static DBusMessage *set_property_request(struct ofono_call_forwarding
*cf,
cf->pending = dbus_message_ref(msg);
cf->query_next = type;
- cf->query_end = type;
+
+ if (type == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
+ ph->number[0] == '\0' && is_cfu_enabled(cf))
+ /*
+ * CFU is removed, conditionals need to be updated
+ */
+ cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
+ else
+ cf->query_end = type;
DBG("Farming off request, will be erasure: %d", ph->number[0] ==
'\0');
--
1.7.5.4
12:46 a.m.
New subject: [PATCHv3 08/12] call-forwarding: Toggle the cached flag on CFU changes
Set the flag on CFU activation, unset on deactivation.
---
src/call-forwarding.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index edd1891..575d96d 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -646,6 +646,14 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
DBG("%s conditions:", cf_type_lut[cf->query_next]);
cf_cond_list_print(cf->cf_conditions[cf->query_next]);
+ /*
+ * Toggle the cached flag on CFU changes
+ */
+ if (cf->query_next == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
+ ((cf->flags & CALL_FORWARDING_FLAG_CACHED) !=
+ is_cfu_enabled(cf)))
+ cf->flags ^= CALL_FORWARDING_FLAG_CACHED;
+
if (cf->query_next == cf->query_end) {
__ofono_dbus_pending_reply(&cf->pending,
dbus_message_new_method_return(cf->pending));
--
1.7.5.4
12:46 a.m.
New subject: [PATCHv3 09/12] call-forwarding: Cache cfs on CFU removal
Since the re-querying is done, cache the call forwardings.
---
src/call-forwarding.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 575d96d..7109c21 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -38,6 +38,7 @@
#define CALL_FORWARDING_FLAG_CACHED 0x1
#define CALL_FORWARDING_FLAG_CPHS_CFF 0x2
+#define CALL_FORWARDING_FLAG_CACHE 0x4
/* According to 27.007 Spec */
#define DEFAULT_NO_REPLY_TIMEOUT 20
@@ -635,7 +636,8 @@ static void set_query_cf_callback(const struct ofono_error *error, int
total,
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
ofono_error("Setting succeeded, but query failed");
- cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
+ cf->flags &= ~(CALL_FORWARDING_FLAG_CACHED |
+ CALL_FORWARDING_FLAG_CACHE);
__ofono_dbus_pending_reply(&cf->pending,
__ofono_error_failed(cf->pending));
return;
@@ -655,6 +657,11 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
cf->flags ^= CALL_FORWARDING_FLAG_CACHED;
if (cf->query_next == cf->query_end) {
+ if (cf->flags & CALL_FORWARDING_FLAG_CACHE) {
+ cf->flags &= ~CALL_FORWARDING_FLAG_CACHE;
+ cf->flags |= CALL_FORWARDING_FLAG_CACHED;
+ }
+
__ofono_dbus_pending_reply(&cf->pending,
dbus_message_new_method_return(cf->pending));
return;
@@ -701,12 +708,13 @@ static DBusMessage *set_property_request(struct
ofono_call_forwarding *cf,
cf->query_next = type;
if (type == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
- ph->number[0] == '\0' && is_cfu_enabled(cf))
+ ph->number[0] == '\0' && is_cfu_enabled(cf)) {
/*
* CFU is removed, conditionals need to be updated
*/
cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
- else
+ cf->flags |= CALL_FORWARDING_FLAG_CACHE;
+ } else
cf->query_end = type;
DBG("Farming off request, will be erasure: %d", ph->number[0] ==
'\0');
--
1.7.5.4
11:16 a.m.
New subject: [PATCHv3 09/12] call-forwarding: Cache cfs on CFU removal
Hi Oleg,
On 03/13/2012 08:46 AM, Oleg Zhurakivskyy wrote:
Since the re-querying is done, cache the call forwardings.
---
src/call-forwarding.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 575d96d..7109c21 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -38,6 +38,7 @@
#define CALL_FORWARDING_FLAG_CACHED 0x1
#define CALL_FORWARDING_FLAG_CPHS_CFF 0x2
+#define CALL_FORWARDING_FLAG_CACHE 0x4
You really have to pick a better name for this flag, I really don't
understand its purpose right now.
/* According to 27.007 Spec */
#define DEFAULT_NO_REPLY_TIMEOUT 20
@@ -635,7 +636,8 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
ofono_error("Setting succeeded, but query failed");
- cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
+ cf->flags &= ~(CALL_FORWARDING_FLAG_CACHED |
+ CALL_FORWARDING_FLAG_CACHE);
__ofono_dbus_pending_reply(&cf->pending,
__ofono_error_failed(cf->pending));
return;
@@ -655,6 +657,11 @@ static void set_query_cf_callback(const struct ofono_error *error,
int total,
cf->flags ^= CALL_FORWARDING_FLAG_CACHED;
if (cf->query_next == cf->query_end) {
+ if (cf->flags & CALL_FORWARDING_FLAG_CACHE) {
+ cf->flags &= ~CALL_FORWARDING_FLAG_CACHE;
+ cf->flags |= CALL_FORWARDING_FLAG_CACHED;
+ }
+
__ofono_dbus_pending_reply(&cf->pending,
dbus_message_new_method_return(cf->pending));
return;
@@ -701,12 +708,13 @@ static DBusMessage *set_property_request(struct
ofono_call_forwarding *cf,
cf->query_next = type;
if (type == CALL_FORWARDING_TYPE_UNCONDITIONAL &&
- ph->number[0] == '\0' && is_cfu_enabled(cf))
+ ph->number[0] == '\0' && is_cfu_enabled(cf)) {
/*
* CFU is removed, conditionals need to be updated
*/
cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
- else
+ cf->flags |= CALL_FORWARDING_FLAG_CACHE;
+ } else
cf->query_end = type;
DBG("Farming off request, will be erasure: %d", ph->number[0] ==
'\0');
Regards,
-Denis
10:51 p.m.
New subject: [PATCHv3 09/12] call-forwarding: Cache cfs on CFU removal
Hello Denis,
On 03/21/2012 02:16 AM, Denis Kenzior wrote:
> +#define CALL_FORWARDING_FLAG_CACHE 0x4
You really have to pick a better name for this flag, I really don't
understand its purpose right now.
The intention of this flag was to distinguish the setting of the single CF
condition from the CFU deactivation were the conditional queries will be re-run
and cached. But, after having another look at this, you are correct, such flag
isn't needed. I will be glad to get rid of it, the naming is really confusing.
Thanks for reviewing and help! I will fix all these and prepare another patchset.
Regards,
Oleg
--
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
12:46 a.m.
New subject: [PATCHv3 10/12] call-forwarding: Re-run ss path cfs queries on cfu changes
Re-run conditional call forwarding queries on cfu removal
via supplementary services path and cache.
---
src/call-forwarding.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 7109c21..b168dc3 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -987,7 +987,8 @@ static void ss_set_query_cf_callback(const struct ofono_error *error,
int total,
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
ofono_error("Setting succeeded, but query failed");
- cf->flags &= ~CALL_FORWARDING_FLAG_CACHED;
+ cf->flags &= ~(CALL_FORWARDING_FLAG_CACHE |
+ CALL_FORWARDING_FLAG_CACHED);
reply = __ofono_error_failed(cf->pending);
__ofono_dbus_pending_reply(&cf->pending, reply);
return;
@@ -1000,6 +1001,10 @@ static void ss_set_query_cf_callback(const struct ofono_error
*error, int total,
cf->ss_req->cf_list[cf->query_next] = l;
if (cf->query_next == cf->query_end) {
+ if (cf->flags & CALL_FORWARDING_FLAG_CACHE) {
+ cf->flags &= ~CALL_FORWARDING_FLAG_CACHE;
+ cf->flags |= CALL_FORWARDING_FLAG_CACHED;
+ }
reply = cf_ss_control_reply(cf, cf->ss_req);
__ofono_dbus_pending_reply(&cf->pending, reply);
g_free(cf->ss_req);
@@ -1177,6 +1182,13 @@ static gboolean cf_ss_control(int type, const char *sc,
cf->pending = dbus_message_ref(msg);
switch (cf->ss_req->cf_type) {
+ case CALL_FORWARDING_TYPE_UNCONDITIONAL:
+ cf->flags |= CALL_FORWARDING_FLAG_CACHE;
+ cf->query_next = CALL_FORWARDING_TYPE_UNCONDITIONAL;
+ cf->query_end = is_cfu_enabled(cf) ?
+ CALL_FORWARDING_TYPE_NOT_REACHABLE :
+ CALL_FORWARDING_TYPE_UNCONDITIONAL;
+ break;
case CALL_FORWARDING_TYPE_ALL:
cf->query_next = CALL_FORWARDING_TYPE_UNCONDITIONAL;
cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
--
1.7.5.4
12:46 a.m.
New subject: [PATCHv3 11/12] call-forwarding: Cache cfs on all/all conditional removal
Re-run conditional call forwarding queries on cfu removal
via supplementary services path and cache.
---
src/call-forwarding.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index b168dc3..edfb87a 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -1181,7 +1181,7 @@ static gboolean cf_ss_control(int type, const char *sc,
cf->pending = dbus_message_ref(msg);
- switch (cf->ss_req->cf_type) {
+ switch (cf_type) {
case CALL_FORWARDING_TYPE_UNCONDITIONAL:
cf->flags |= CALL_FORWARDING_FLAG_CACHE;
cf->query_next = CALL_FORWARDING_TYPE_UNCONDITIONAL;
@@ -1190,11 +1190,11 @@ static gboolean cf_ss_control(int type, const char *sc,
CALL_FORWARDING_TYPE_UNCONDITIONAL;
break;
case CALL_FORWARDING_TYPE_ALL:
- cf->query_next = CALL_FORWARDING_TYPE_UNCONDITIONAL;
- cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
- break;
case CALL_FORWARDING_TYPE_ALL_CONDITIONAL:
- cf->query_next = CALL_FORWARDING_TYPE_BUSY;
+ cf->flags |= CALL_FORWARDING_FLAG_CACHE;
+ cf->query_next = (cf_type == CALL_FORWARDING_TYPE_ALL) ?
+ CALL_FORWARDING_TYPE_UNCONDITIONAL :
+ CALL_FORWARDING_TYPE_BUSY;
cf->query_end = CALL_FORWARDING_TYPE_NOT_REACHABLE;
break;
default:
--
1.7.5.4
12:46 a.m.
New subject: [PATCHv3 12/12] TODO: Remove completed call forwarding state task
---
TODO | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)
diff --git a/TODO b/TODO
index 1fef651..9948117 100644
--- a/TODO
+++ b/TODO
@@ -213,23 +213,6 @@ Supplementary Services
Priority: Low
Complexity: C8
-- Call forwarding state handling change
-
- At the moment call forwarding states are not always correct. Any active
- conditional call forwarding should become quiescent while unconditional call
- forwarding is activate. If call forwarding unconditional is subsequently
- deactivated, all the quiescent forwardings should become operative again.
- I.e. No conditional call forwarding string should be returned while
- unconditional call forwarding is active even if they exist.
-
- If there is an successful attempt to activate/deactivate conditional call
- forwarding while unconditional call forwarding is active the conditional cache
- flag should cleared.
-
- Priority: High
- Complexity: C1
- Owner: Nicolas Bertrand <nicolas.bertrand(a)linux.intel.com>
-
Voicecall
=========
--
1.7.5.4
3833
days inactive
3841
days old
24 comments
2 participants
participants (2)
-
Denis Kenzior -
Oleg Zhurakivskyy