Re: Question about static analysis

[Slava Monich]

Hi everyone

Can I inquire here? If it's a problem, I'll delete it.

I have question about static analysis. below code, Why use after free?
I can see modem.c and ussd.c,  simfs.c let me know if you have any intentions.


g_slist_remove() doesn't actually access the pointer. It doesn't even 
assume that it's a valid pointer (it could be an int cast to a pointer, 
for example). There's no danger whatsoever in freeing the memory first 
and then passing the pointer to g_slist_remove()


Although it would probably be cleaner to use g_slist_delete_link() for 
the found link, instead of g_slist_remove()


Cheers,

-Slava



=
Use after free (USE_AFTER_FREE)
pass_freed_arg: Passing freed pointer found->data as an argument to 
g_slist_remove

in modem.c
g_free(found->data);
modem->interface_list = 
g_slist_remove(modem->interface_list,found->data);

feature = get_feature(interface);
if (feature) {
found = g_slist_find_custom(modem->feature_list, 
feature,(GCompareFunc) strcmp);

if (found) {
g_free(found->data);
modem->feature_list =
g_slist_remove(modem->feature_list,  
found->data);
}
}
=
in ussd.c
ssc_entry_destroy(l->data);
ussd->ss_control_list = g_slist_remove(ussd->ss_control_list, l->data);
=   
in simfs.c
sim_fs_op_free(op);
g_queue_remove(fs->op_q, op);
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org
.

___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

Re: [PATCH] simfs: Fix reads beyond the first block

[Slava Monich]

On 30/07/2021 18.37, Denis Kenzior wrote:

Hi Slava,

On 7/30/21 7:07 AM, Slava Monich wrote:

---
  src/simfs.c | 17 -
  1 file changed, 8 insertions(+), 9 deletions(-)



Funny how long this bug has been lurking around.



Until we finally had a crash on reading an icon or something out of SIM. 
Which most SIMs apparently don't have or else it would've been noticed 
earlier.




diff --git a/src/simfs.c b/src/simfs.c
index 3d4f6283..cf770265 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -383,18 +383,18 @@ static void sim_fs_op_read_block_cb(const 
struct ofono_error *error,

  }
    start_block = op->offset / 256;
-    end_block = (op->offset + (op->num_bytes - 1)) / 256;
+    end_block = op->num_bytes ? (op->offset + op->num_bytes - 1) / 
256 :

+    start_block;


Curious why this is needed?  op->num_bytes should never be zero since 
it gets set to the file length?




I admit that it's a bit paranoid, but op->num_bytes is assigned without 
checking and I figured that it wouldn't hurt to do a check here. Feel 
free to drop this part if it looks like too much of an overkill to you.




Rest looks good to me.

Regards,
-Denis



Cheers,

-Slava
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH] simfs: Fix reads beyond the first block

[Slava Monich]

---
 src/simfs.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/src/simfs.c b/src/simfs.c
index 3d4f6283..cf770265 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -383,18 +383,18 @@ static void sim_fs_op_read_block_cb(const struct 
ofono_error *error,
}
 
start_block = op->offset / 256;
-   end_block = (op->offset + (op->num_bytes - 1)) / 256;
+   end_block = op->num_bytes ? (op->offset + op->num_bytes - 1) / 256 :
+   start_block;
 
if (op->current == start_block) {
bufoff = 0;
dataoff = op->offset % 256;
-   tocopy = MIN(256 - op->offset % 256,
-   op->num_bytes - op->current * 256);
+   tocopy = MIN(256 - dataoff, op->num_bytes);
} else {
-   bufoff = (op->current - start_block - 1) * 256 +
+   bufoff = (op->current - start_block) * 256 -
op->offset % 256;
dataoff = 0;
-   tocopy = MIN(256, op->num_bytes - op->current * 256);
+   tocopy = MIN(256, op->num_bytes - bufoff);
}
 
DBG("bufoff: %d, dataoff: %d, tocopy: %d",
@@ -463,13 +463,12 @@ static gboolean sim_fs_op_read_block(gpointer user_data)
bufoff = 0;
seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256 +
op->offset % 256;
-   toread = MIN(256 - op->offset % 256,
-   op->num_bytes - op->current * 256);
+   toread = MIN(256 - op->offset % 256, op->num_bytes);
} else {
-   bufoff = (op->current - start_block - 1) * 256 +
+   bufoff = (op->current - start_block) * 256 -
op->offset % 256;
seekoff = SIM_CACHE_HEADER_SIZE + op->current * 256;
-   toread = MIN(256, op->num_bytes - op->current * 256);
+   toread = MIN(256, op->num_bytes - bufoff);
}
 
DBG("bufoff: %d, seekoff: %d, toread: %d",
-- 
2.25.1
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH] sim-auth: Parse auth response according to TS 31.102

[Slava Monich]

---
 src/sim-auth.c  |  34 ++-
 src/simutil.c   | 146 +---
 src/simutil.h   |  13 ++--
 unit/test-simutil.c | 125 -
 4 files changed, 234 insertions(+), 84 deletions(-)

diff --git a/src/sim-auth.c b/src/sim-auth.c
index 3c3f35e7..6dab52ee 100644
--- a/src/sim-auth.c
+++ b/src/sim-auth.c
@@ -207,14 +207,10 @@ static void handle_umts(struct ofono_sim_auth *sa, const 
uint8_t *resp,
DBusMessage *reply = NULL;
DBusMessageIter iter;
DBusMessageIter dict;
-   const uint8_t *res = NULL;
-   const uint8_t *ck = NULL;
-   const uint8_t *ik = NULL;
-   const uint8_t *auts = NULL;
-   const uint8_t *kc = NULL;
+   struct data_block res, ck, ik, auts, sres, kc;
 
if (!sim_parse_umts_authenticate(resp, len, , , ,
-   , ))
+   , , ))
goto umts_end;
 
reply = dbus_message_new_method_return(sa->pending->msg);
@@ -224,15 +220,23 @@ static void handle_umts(struct ofono_sim_auth *sa, const 
uint8_t *resp,
dbus_message_iter_open_container(, DBUS_TYPE_ARRAY,
"{say}", );
 
-   if (auts) {
-   append_dict_byte_array(, "AUTS", auts, 14);
-   } else {
-   append_dict_byte_array(, "RES", res, 8);
-   append_dict_byte_array(, "CK", ck, 16);
-   append_dict_byte_array(, "IK", ik, 16);
-   if (kc)
-   append_dict_byte_array(, "Kc", kc, 8);
-   }
+   if (auts.data)
+   append_dict_byte_array(, "AUTS", auts.data, auts.len);
+
+   if (sres.data)
+   append_dict_byte_array(, "SRES", sres.data, sres.len);
+
+   if (res.data)
+   append_dict_byte_array(, "RES", res.data, res.len);
+
+   if (ck.data)
+   append_dict_byte_array(, "CK", ck.data, ck.len);
+
+   if (ik.data)
+   append_dict_byte_array(, "IK", ik.data, ik.len);
+
+   if (kc.data)
+   append_dict_byte_array(, "Kc", kc.data, kc.len);
 
dbus_message_iter_close_container(, );
 
diff --git a/src/simutil.c b/src/simutil.c
index 5d2aa6a2..30b76f05 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -1672,63 +1672,135 @@ int sim_build_gsm_authenticate(unsigned char *buffer, 
int len,
return build_authenticate(buffer, rand, NULL);
 }
 
-gboolean sim_parse_umts_authenticate(const unsigned char *buffer,
-   int len, const unsigned char **res, const unsigned char **ck,
-   const unsigned char **ik, const unsigned char **auts,
-   const unsigned char **kc)
+gboolean sim_parse_umts_authenticate(const unsigned char *buffer, int len,
+   struct data_block *res, struct data_block *ck,
+   struct data_block *ik, struct data_block *auts,
+   struct data_block *sres, struct data_block *kc)
 {
-   if (len < 16 || !buffer)
+   const unsigned char *ptr = buffer;
+   const unsigned char *end = ptr + len;
+   unsigned int l;
+
+   if (!buffer || len < 2)
return FALSE;
 
-   switch (buffer[0]) {
+   memset(res, 0, sizeof(*res));
+   memset(ck, 0, sizeof(*ck));
+   memset(ik, 0, sizeof(*ik));
+   memset(kc, 0, sizeof(*kc));
+   memset(auts, 0, sizeof(*auts));
+   memset(sres, 0, sizeof(*sres));
+
+   /*
+* TS 31.102
+* 7.1.2.1 GSM/3G security context
+*/
+   switch (*ptr++) {
case 0xdb:
-   /* 'DB' + '08' + RES(16) + '10' + CK(32) + '10' + IK(32) = 43 */
-   if (len < 43)
-   goto umts_end;
+   /*
+* Response parameters/data, case 1, 3G security context,
+* command successful:
+*
+* "Successful 3G authentication" tag = 'DB'
+* 'DB' + L3 + RES(L3) + L4 + CK(L4) + L5 + IK(L5) + 8 + Kc(8)
+*/
+   l = *ptr++; /* L3 */
+   if ((ptr + l) > end)
+   return FALSE;
 
-   /* success */
-   if (buffer[1] != 0x08)
-   goto umts_end;
+   res->data = ptr;
+   res->len = l;
+   ptr += l;
 
-   *res = buffer + 2;
+   if (ptr == end)
+   return FALSE;
 
-   if (buffer[10] != 0x10)
-   goto umts_end;
+   l = *ptr++; /* L4 */
+   if ((ptr + l) > end)
+   return FALSE;
 
-   *ck = buffer + 11;
+   ck->data = ptr;
+   ck->len = l;
+   ptr += l;
 
-   if (buffer[27] != 0x10)
-   goto umts_end;
+   if (ptr == end)
+   return FALSE;
 
-   *ik = buffer + 28;
+   l = *ptr++; /* L5 */
+  

Re: [PATCH] simutil: Fill unused part of AID with FFs

[Slava Monich]

On 30.4.2021 17.54, Denis Kenzior wrote:

Hi Slava,

On 4/29/21 11:09 AM, Slava Monich wrote:

Correct handling of short AIDs will take more than that, but
leaving part of the array uninitialized is wrong in any case.
---
  src/simutil.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/src/simutil.c b/src/simutil.c
index 5d2aa6a2..e648c918 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -1588,6 +1588,7 @@ GSList *sim_parse_app_template_entries(const 
unsigned char *buffer, int len)

  goto error;
    memcpy(app.aid, aid, app.aid_len);
+    memset(app.aid + app.aid_len, 0xff, 16 - app.aid_len);


Would it not be easier to fix sim-auth to take aid_len into account 
instead of hard-coding 16?  It seems like sim_auth_register is the 
only one affected, but I didn't look deeply.



AFAICT it's not that trivial but feel free to disregard this patch - it 
doesn't make much sense to fix it half-way.



Rebards,

-Slava
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH] simutil: Fill unused part of AID with FFs

[Slava Monich]

Correct handling of short AIDs will take more than that, but
leaving part of the array uninitialized is wrong in any case.
---
 src/simutil.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/simutil.c b/src/simutil.c
index 5d2aa6a2..e648c918 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -1588,6 +1588,7 @@ GSList *sim_parse_app_template_entries(const unsigned 
char *buffer, int len)
goto error;
 
memcpy(app.aid, aid, app.aid_len);
+   memset(app.aid + app.aid_len, 0xff, 16 - app.aid_len);
 
app.type = (app.aid[5] << 8) | app.aid[6];
 
-- 
2.25.1
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH 2/2] sim-auth: Only close open sessions

[Slava Monich]

Session has to be open in order to have a valid session_id
---
 src/sim.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/sim.c b/src/sim.c
index 793ff64a..3319ecee 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -3805,7 +3805,8 @@ void __ofono_sim_remove_session_watch(struct 
ofono_sim_aid_session *session,
 {
__ofono_watchlist_remove_item(session->watches, id);
 
-   if (g_slist_length(session->watches->items) == 0) {
+   if (g_slist_length(session->watches->items) == 0 &&
+   session->state == SESSION_STATE_OPEN) {
/* last watcher, close session */
session->state = SESSION_STATE_CLOSING;
session->sim->driver->close_channel(session->sim,
-- 
2.25.1
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH 1/2] sim-auth: Remove watch if open_channel fails

[Slava Monich]

Otherwise open_channel won't be called again after a failure.
---
 src/sim-auth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/sim-auth.c b/src/sim-auth.c
index 0e13b533..3c3f35e7 100644
--- a/src/sim-auth.c
+++ b/src/sim-auth.c
@@ -367,6 +367,8 @@ static void get_session_cb(ofono_bool_t active, int 
session_id,
 error:
__ofono_dbus_pending_reply(>pending->msg,
__ofono_error_failed(sa->pending->msg));
+   __ofono_sim_remove_session_watch(sa->pending->session,
+   sa->pending->watch_id);
g_free(sa->pending);
sa->pending = NULL;
 }
-- 
2.25.1
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH] lte: Use the right D-Bus interface for property change signal

[Slava Monich]

---
 src/lte.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lte.c b/src/lte.c
index fbe0116..951a06f 100644
--- a/src/lte.c
+++ b/src/lte.c
@@ -212,7 +212,7 @@ static void lte_set_default_attach_info_cb(const struct 
ofono_error *error,
}
 
ofono_dbus_signal_property_changed(conn, path,
-   OFONO_CONNECTION_CONTEXT_INTERFACE,
+   OFONO_LTE_INTERFACE,
key,
DBUS_TYPE_STRING, );
 
-- 
1.9.1
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

[PATCH] cbs: Allow the last CBS fragment to be truncated

[Slava Monich]

That does happen in real life.
---
 src/smsutil.c | 23 +++
 src/smsutil.h |  1 +
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index 8c084d4..450a1d3 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -1756,7 +1756,7 @@ gboolean sms_udh_iter_init_from_cbs(const struct cbs *cbs,
return FALSE;
 
hdr = cbs->ud;
-   max_ud_len = 82;
+   max_ud_len = cbs->udlen;
 
/* Must have at least one information-element if udhi is true */
if (hdr[0] < 2)
@@ -3862,8 +3862,8 @@ gboolean cbs_dcs_decode(guint8 dcs, gboolean *udhi, enum 
sms_class *cls,
 
 gboolean cbs_decode(const unsigned char *pdu, int len, struct cbs *out)
 {
-   /* CBS is always a fixed length of 88 bytes */
-   if (len != 88)
+   /* CBS is (almost) always a fixed length of 88 bytes */
+   if (len < 6 || len > 88)
return FALSE;
 
out->gs = (enum cbs_geo_scope) ((pdu[0] >> 6) & 0x03);
@@ -3874,6 +3874,10 @@ gboolean cbs_decode(const unsigned char *pdu, int len, 
struct cbs *out)
out->max_pages = pdu[5] & 0xf;
out->page = (pdu[5] >> 4) & 0xf;
 
+   /* Allow the last fragment to be truncated */
+   if (len != 88 && out->max_pages != out->page)
+   return FALSE;
+
/*
 * If a mobile receives the code  in either the first field or
 * the second field then it shall treat the CBS message exactly the
@@ -3885,7 +3889,10 @@ gboolean cbs_decode(const unsigned char *pdu, int len, 
struct cbs *out)
out->page = 1;
}
 
-   memcpy(out->ud, pdu + 6, 82);
+   out->udlen = (guint8)(len - 6);
+   memcpy(out->ud, pdu + 6, out->udlen);
+   if (out->udlen < 82)
+   memset(out->ud + out->udlen, 0, 82 - out->udlen);
 
return TRUE;
 }
@@ -4078,7 +4085,7 @@ char *cbs_decode_text(GSList *cbs_list, char *iso639_lang)
if (iso639)
bufsize -= 3;
} else {
-   bufsize += 82;
+   bufsize += cbs->udlen;
 
if (iso639)
bufsize -= 2;
@@ -4095,7 +4102,7 @@ char *cbs_decode_text(GSList *cbs_list, char *iso639_lang)
if (sms_udh_iter_init_from_cbs(cbs, ))
taken = sms_udh_iter_get_udh_length() + 1;
 
-   unpack_7bit_own_buf(cbs->ud + taken, 82 - taken,
+   unpack_7bit_own_buf(cbs->ud + taken, cbs->udlen - taken,
taken, false, 2,
NULL, 0,
(unsigned char *)iso639_lang);
@@ -4128,7 +4135,7 @@ char *cbs_decode_text(GSList *cbs_list, char *iso639_lang)
max_chars =
sms_text_capacity_gsm(CBS_MAX_GSM_CHARS, taken);
 
-   unpack_7bit_own_buf(ud + taken, 82 - taken,
+   unpack_7bit_own_buf(ud + taken, cbs->udlen - taken,
taken, false, max_chars,
, 0, unpacked);
 
@@ -4162,7 +4169,7 @@ char *cbs_decode_text(GSList *cbs_list, char *iso639_lang)
 * the check here since the specification isn't clear
 */
} else {
-   int num_ucs2_chars = (82 - taken) >> 1;
+   int num_ucs2_chars = (cbs->udlen - taken) >> 1;
int i = taken;
int max_offset = taken + num_ucs2_chars * 2;
 
diff --git a/src/smsutil.h b/src/smsutil.h
index 0adba51..01487de 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -408,6 +408,7 @@ struct cbs {
guint8 dcs; /* 8 bits */
guint8 max_pages;   /* 4 bits */
guint8 page;/* 4 bits */
+   guint8 udlen;
guint8 ud[82];
 };
 
-- 
1.9.1
___
ofono mailing list -- ofono@ofono.org
To unsubscribe send an email to ofono-le...@ofono.org

Re: [PATCH] src/modem: connection timeout to 60 seconds

[Slava Monich]

On 23/10/18 10:50, Giacinto Cifelli wrote:

Changed the connection timeout to 60 seconds (from 20 seconds),
to accomodate for chipset that take time to boot-up.


Is it hard to make it configurable?

Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH v7 6/7] gemalto/lte: Gemalto vendor lte atom

[Slava Monich]

On 18/10/18 22:32, Jonas Bonn wrote:

...


+#include 
+#include 
+
+#include "ofono.h"
+#include "gemaltomodem.h"
+
+enum gemalto_auth_option {
+    GEMALTO_AUTH_DEFAULTS    = 0x00,
+    GEMALTO_AUTH_USE_SGAUTH    = 0x01,
+    GEMALTO_AUTH_ORDER_PWD_USR    = 0x02,
+    GEMALTO_AUTH_ALW_ALL_PARAMS    = 0x04,
+};


Using an enumeration to name flags feels awkward.  I would #define these:

#define AUTH_F_USE_SGATH (1<<0)
#define AUTH_F_SWAP_CREDENTIALS (1<<1)
#define AUTH_F_ALWAYS_ALL_PARAMS (1<<2)




It may feel awkward but I actually find that (enum'ing flags) quite 
useful for debugging with gdb - it allows the debugger to refer to those 
flags by name.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] extended support for LTE and NR. Some minor fixes. part 1 of 7

[Slava Monich]

On 20/09/18 00:45, Denis Kenzior wrote:
(ANY = PAP_CHAP, and don't ask me why we added new values to the 
beginning of the enum - it was before we started using binary 
plugins). I would be more than happy if upstream started to use the 
same enum!




That assumes that we should support your METHOD_ANY thing.  I've not 
heard any good arguments for that yet...


At least one Chinese modem I dealt with had AT+ EGPAU = ,, 
 where  is 0 for PAP, 1 for CHAP, 2 for NONE and 3 for 
PAP_CHAP. Also, Android RIL interface has value 3 reserved for PAP 
(see ril.h for older Androids and ApnAuthType in binder radio interface 
for Android 8+). To me that sounds like a valid use case.


-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] extended support for LTE and NR. Some minor fixes. part 1 of 7

[Slava Monich]

On 19/09/18 19:32, Denis Kenzior wrote:

Hi Slava,

Anything in include is external API. We do maintain not just 
out-of-tree, but binary plugins. Backward (binary!) compatibility a 
must for us. Please do not break it.


That is why we have 'OFONO_API_SUBJECT_TO_CHANGE' as a reminder. We 
mean it.  D-Bus API is stable, we never made any guarantees about the 
internal APIs.




I understand that! It makes things easier for you guys.

But we had to avoid certain compile-time dependencies in ofono, and a 
straightforward (and perhaps the only) way to achieve that was to use 
binary plugins. For us plugin API is not subject to change (plugins 
don't necessarily get upgraded together with ofono), meaning more 
changes between our fork and upstream in case if upstream breaks it, 
more maintenance work and more room for errors. Obviously, I would like 
to keep differences to a minimum.


I'm just humbly asking - if there's a way to keep plugin API backward 
compatible, please do it that way. There is at least one person in the 
world who would appreciate it.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] extended support for LTE and NR. Some minor fixes. part 1 of 7

[Slava Monich]

On 19/09/18 18:21, Denis Kenzior wrote:


Hi Giacinto,

I would favour also renumbering with NONE on top, but I am not sure 
of the side effects everywhere, in case the values are used directly 
in commands.


Actually there is no problem doing that.  The internal API is not 
stable.  Besides, it will give all the guys who insist on maintaining 
out-of-tree plugins some extra work ;)




Anything in include is external API. We do maintain not just 
out-of-tree, but binary plugins. Backward (binary!) compatibility a must 
for us. Please do not break it.


-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] extended support for LTE and NR. Some minor fixes. part 1 of 7

[Slava Monich]

On 19/09/18 08:37, Giacinto Cifelli wrote:

---
  include/gprs-context.h |  1 +
  include/lte.h  | 11 +--
  2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/gprs-context.h b/include/gprs-context.h
index 20ca9ef..8869c12 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -57,6 +57,7 @@ enum ofono_gprs_context_type {
  enum ofono_gprs_auth_method {
OFONO_GPRS_AUTH_METHOD_CHAP = 0,
OFONO_GPRS_AUTH_METHOD_PAP,
+   OFONO_GPRS_AUTH_METHOD_NONE,


I think there should be OFONO_GPRS_AUTH_METHOD_ANY (or 
OFONO_GPRS_AUTH_METHOD_PAP_CHAP) here as well, for completeness. Many 
modems support that too (and we had to add it in our fork).




  };
  
  struct ofono_gprs_primary_context {

diff --git a/include/lte.h b/include/lte.h
index ef84ab9..38587c3 100644
--- a/include/lte.h
+++ b/include/lte.h
@@ -3,6 +3,7 @@
   *  oFono - Open Source Telephony
   *
   *  Copyright (C) 2016  Endocode AG. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
   *
   *  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
@@ -28,14 +29,18 @@ extern "C" {
  
  #include 
  
-struct ofono_lte;

-
  struct ofono_lte_default_attach_info {
char apn[OFONO_GPRS_MAX_APN_LENGTH + 1];
+   enum ofono_gprs_proto proto;
+   enum ofono_gprs_auth_method auth_method;
+   char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
+   char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
  };


This is starting to look suspiciously similar to struct 
ofono_gprs_primary_context (the only thing left is cid). Is it really 
necessary to maintain two copies of essentially the same structure or is 
there some room for unification here?


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] atmodem: add LTE state for AT+CREG

[Slava Monich]

Hi Denis,

Hi Slava,

On 09/10/2018 01:26 PM, Slava Monich wrote:

Hi Anirudh, Denis at al.

@@ -669,6 +669,10 @@ const char *registration_status_to_string(int 
status)

  return "unknown";
  case NETWORK_REGISTRATION_STATUS_ROAMING:
  return "roaming";
+    case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+    return "registered lte";
+    case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+    return "roaming lte";


What would be the difference between "roaming lte" and "roaming" (or 
"registered lte" vs "registered") from the viewpoint of D-Bus API 
clients? In other words, what would e.g. dialer do differently 
depending on whether registration status is "registered lte" or just 
"registered"? Is it worth the API break? The existing clients won't 
know how to handle the new values until they (clients) get updated.




I agree.  That is why I suggested a separate Property for this (and 
the CSFB case from 27.007) instead of modifying the Status property 
directly.


E.g. REGISTERED_SMS_EUTRAN would map to Status="registered" and 
SmsOnly=False.  ROAMING_SMS_EUTRAN would map to Status="roaming" and 
SmsOnly=True.


I assume that the Dialer can ignore this as the modem will fall back 
to 3G in case of a call?  Someone feel free to educate me.


The modems I dealt with do drop to 3G for voice calls. Once voice call 
terminates, they switch back to LTE (well, most of the time). Dialler 
(at least our dialler) doesn't care about access technology.


Just a thought - could the presence of org.ofono.VoiceCallManager and 
org.ofono.MessageManager interfaces be used to indicate whether voice 
calls and/or messaging is available? That wouldn't require any D-Bus API 
changes at all.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] atmodem: add LTE state for AT+CREG

[Slava Monich]

Hi Anirudh, Denis at al.

@@ -669,6 +669,10 @@ const char *registration_status_to_string(int 
status)

  return "unknown";
  case NETWORK_REGISTRATION_STATUS_ROAMING:
  return "roaming";
+    case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+    return "registered lte";
+    case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+    return "roaming lte";


What's would be the difference between "roaming lte" and "roaming" (or 
"registered lte" vs "registered") from the viewpoint of D-Bus API 
clients? In other words, what would e.g. dialer do differently depending 
on whether registration status is "registered lte" or just "registered"? 
Is it worth the API break? The existing clients won't know how to handle 
the new values until they (clients) get updated.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/2] include: Add ofono_modem_get_voicecall

[Slava Monich]

On 02/07/18 18:34, Denis Kenzior wrote:

Hi Slava,

On 06/29/2018 08:57 AM, Slava Monich wrote:

---
  include/modem.h | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/include/modem.h b/include/modem.h
index 005a42e..bed46c2 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -31,6 +31,7 @@ extern "C" {
  struct ofono_modem;
  struct ofono_gprs;
  struct ofono_sim;
+struct ofono_voicecall;
    enum ofono_modem_type {
  OFONO_MODEM_TYPE_HARDWARE = 0,
@@ -84,6 +85,7 @@ void ofono_modem_remove_interface(struct 
ofono_modem *modem,

  const char *ofono_modem_get_path(struct ofono_modem *modem);
  struct ofono_sim *ofono_modem_get_sim(struct ofono_modem *modem);
  struct ofono_gprs *ofono_modem_get_gprs(struct ofono_modem *modem);
+struct ofono_voicecall *ofono_modem_get_voicecall(struct ofono_modem 
*modem);

    void ofono_modem_set_data(struct ofono_modem *modem, void *data);
  void *ofono_modem_get_data(struct ofono_modem *modem);



I went ahead and applied this one.  But if we need any more of these I 
would seriously consider exposing __ofono_atom_find to the public API 
instead.





I kind of like these getters for type safety but it's all up to you of 
course.


Regards,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 2/2] modem: Implement ofono_modem_get_voicecall

[Slava Monich]

---
 src/modem.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/modem.c b/src/modem.c
index 9409347..9e25448 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -194,6 +194,11 @@ struct ofono_gprs *ofono_modem_get_gprs(struct ofono_modem 
*modem)
return __ofono_atom_find(OFONO_ATOM_TYPE_GPRS, modem);
 }
 
+struct ofono_voicecall *ofono_modem_get_voicecall(struct ofono_modem *modem)
+{
+   return __ofono_atom_find(OFONO_ATOM_TYPE_VOICECALL, modem);
+}
+
 struct ofono_atom *__ofono_modem_add_atom(struct ofono_modem *modem,
enum ofono_atom_type type,
void (*destruct)(struct ofono_atom *),
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 1/2] include: Add ofono_modem_get_voicecall

[Slava Monich]

---
 include/modem.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/modem.h b/include/modem.h
index 005a42e..bed46c2 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -31,6 +31,7 @@ extern "C" {
 struct ofono_modem;
 struct ofono_gprs;
 struct ofono_sim;
+struct ofono_voicecall;
 
 enum ofono_modem_type {
OFONO_MODEM_TYPE_HARDWARE = 0,
@@ -84,6 +85,7 @@ void ofono_modem_remove_interface(struct ofono_modem *modem,
 const char *ofono_modem_get_path(struct ofono_modem *modem);
 struct ofono_sim *ofono_modem_get_sim(struct ofono_modem *modem);
 struct ofono_gprs *ofono_modem_get_gprs(struct ofono_modem *modem);
+struct ofono_voicecall *ofono_modem_get_voicecall(struct ofono_modem *modem);
 
 void ofono_modem_set_data(struct ofono_modem *modem, void *data);
 void *ofono_modem_get_data(struct ofono_modem *modem);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 3/3] dbus: Add D-Bus mapping for OFONO_ERROR_TYPE_ERRNO

[Slava Monich]

---
 src/dbus.c | 38 +++---
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/src/dbus.c b/src/dbus.c
index cadf5c6..a175178 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -24,6 +24,7 @@
 #endif
 
 #include 
+#include 
 #include 
 
 #include "ofono.h"
@@ -45,6 +46,16 @@ static const struct error_mapping_entry cme_errors_mapping[] 
= {
{ 31,   __ofono_error_timed_out },
{ 32,   __ofono_error_access_denied },
{ 50,   __ofono_error_invalid_args },
+   { }
+};
+
+static const struct error_mapping_entry errno_errors_mapping[] = {
+   { EACCES,  __ofono_error_access_denied },
+   { EOPNOTSUPP,  __ofono_error_not_supported },
+   { ENOSYS,  __ofono_error_not_implemented },
+   { ETIMEDOUT,   __ofono_error_timed_out },
+   { EINPROGRESS, __ofono_error_busy },
+   { }
 };
 
 static void append_variant(DBusMessageIter *iter,
@@ -419,26 +430,31 @@ DBusMessage *__ofono_error_network_terminated(DBusMessage 
*msg)
" network");
 }
 
-DBusMessage *__ofono_error_from_error(const struct ofono_error *error,
-   DBusMessage *msg)
+static DBusMessage *__ofono_map_error(const struct error_mapping_entry *map,
+   int error, DBusMessage *msg)
 {
const struct error_mapping_entry *e;
-   int maxentries;
-   int i;
 
+   for (e = map; e->ofono_error_func; e++)
+   if (e->error == error)
+   return e->ofono_error_func(msg);
+
+   return __ofono_error_failed(msg);
+}
+
+DBusMessage *__ofono_error_from_error(const struct ofono_error *error,
+   DBusMessage *msg)
+{
switch (error->type) {
case OFONO_ERROR_TYPE_CME:
-   e = cme_errors_mapping;
-   maxentries = sizeof(cme_errors_mapping) /
-   sizeof(struct error_mapping_entry);
-   for (i = 0; i < maxentries; i++)
-   if (e[i].error == error->error)
-   return e[i].ofono_error_func(msg);
-   break;
+   return __ofono_map_error(cme_errors_mapping, error->error, msg);
case OFONO_ERROR_TYPE_CMS:
return __ofono_error_failed(msg);
case OFONO_ERROR_TYPE_CEER:
return __ofono_error_failed(msg);
+   case OFONO_ERROR_TYPE_ERRNO:
+   return __ofono_map_error(errno_errors_mapping,
+   ABS(error->error), msg);
default:
return __ofono_error_failed(msg);
}
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 2/3] emulator: Handle OFONO_ERROR_TYPE_ERRNO in switch

[Slava Monich]

---
 src/emulator.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/emulator.c b/src/emulator.c
index ccb26dc..b3afb3d 100644
--- a/src/emulator.c
+++ b/src/emulator.c
@@ -1347,6 +1347,7 @@ void ofono_emulator_send_final(struct ofono_emulator *em,
case OFONO_ERROR_TYPE_CEER:
case OFONO_ERROR_TYPE_SIM:
case OFONO_ERROR_TYPE_FAILURE:
+   case OFONO_ERROR_TYPE_ERRNO:
 failure:
g_at_server_send_final(em->server, G_AT_SERVER_RESULT_ERROR);
break;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 1/3] include: Add OFONO_ERROR_TYPE_ERRNO

[Slava Monich]

---
 include/types.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/types.h b/include/types.h
index 2c64b20..90d8c2c 100644
--- a/include/types.h
+++ b/include/types.h
@@ -56,6 +56,7 @@ enum ofono_error_type {
OFONO_ERROR_TYPE_CEER,
OFONO_ERROR_TYPE_SIM,
OFONO_ERROR_TYPE_FAILURE,
+   OFONO_ERROR_TYPE_ERRNO
 };
 
 enum ofono_disconnect_reason {
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] dbus: Make cme_errors_mapping static const

[Slava Monich]

---
 src/dbus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/dbus.c b/src/dbus.c
index 3e1e162..cadf5c6 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -37,7 +37,7 @@ struct error_mapping_entry {
DBusMessage *(*ofono_error_func)(DBusMessage *);
 };
 
-struct error_mapping_entry cme_errors_mapping[] = {
+static const struct error_mapping_entry cme_errors_mapping[] = {
{ 3,__ofono_error_not_allowed },
{ 4,__ofono_error_not_supported },
{ 16,   __ofono_error_incorrect_password },
@@ -422,7 +422,7 @@ DBusMessage *__ofono_error_network_terminated(DBusMessage 
*msg)
 DBusMessage *__ofono_error_from_error(const struct ofono_error *error,
DBusMessage *msg)
 {
-   struct error_mapping_entry *e;
+   const struct error_mapping_entry *e;
int maxentries;
int i;
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] dbus: Add D-Bus mapping for generic errors

[Slava Monich]

This allows plugins/drivers to be a bit more specific about
what went wrong.
---
 src/dbus.c | 28 
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/dbus.c b/src/dbus.c
index 3e1e162..7ea86ed 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -24,6 +24,7 @@
 #endif
 
 #include 
+#include 
 #include 
 
 #include "ofono.h"
@@ -37,7 +38,7 @@ struct error_mapping_entry {
DBusMessage *(*ofono_error_func)(DBusMessage *);
 };
 
-struct error_mapping_entry cme_errors_mapping[] = {
+static const struct error_mapping_entry cme_errors_mapping[] = {
{ 3,__ofono_error_not_allowed },
{ 4,__ofono_error_not_supported },
{ 16,   __ofono_error_incorrect_password },
@@ -47,6 +48,14 @@ struct error_mapping_entry cme_errors_mapping[] = {
{ 50,   __ofono_error_invalid_args },
 };
 
+static const struct error_mapping_entry generic_errors_mapping[] = {
+   { EACCES,  __ofono_error_access_denied },
+   { EOPNOTSUPP,  __ofono_error_not_supported },
+   { ENOSYS,  __ofono_error_not_implemented },
+   { ETIMEDOUT,   __ofono_error_timed_out },
+   { EINPROGRESS, __ofono_error_busy }
+};
+
 static void append_variant(DBusMessageIter *iter,
int type, const void *value)
 {
@@ -422,15 +431,14 @@ DBusMessage *__ofono_error_network_terminated(DBusMessage 
*msg)
 DBusMessage *__ofono_error_from_error(const struct ofono_error *error,
DBusMessage *msg)
 {
-   struct error_mapping_entry *e;
+   const struct error_mapping_entry *e;
int maxentries;
int i;
 
switch (error->type) {
case OFONO_ERROR_TYPE_CME:
e = cme_errors_mapping;
-   maxentries = sizeof(cme_errors_mapping) /
-   sizeof(struct error_mapping_entry);
+   maxentries = G_N_ELEMENTS(cme_errors_mapping);
for (i = 0; i < maxentries; i++)
if (e[i].error == error->error)
return e[i].ofono_error_func(msg);
@@ -439,6 +447,18 @@ DBusMessage *__ofono_error_from_error(const struct 
ofono_error *error,
return __ofono_error_failed(msg);
case OFONO_ERROR_TYPE_CEER:
return __ofono_error_failed(msg);
+   case OFONO_ERROR_TYPE_FAILURE:
+   if (error->error) {
+   int err = error->error;
+
+   if (err < 0) err = -err;
+   e = generic_errors_mapping;
+   maxentries = G_N_ELEMENTS(generic_errors_mapping);
+   for (i = 0; i < maxentries; i++)
+   if (e[i].error == err)
+   return e[i].ofono_error_func(msg);
+   }
+   break;
default:
return __ofono_error_failed(msg);
}
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 01/12] include: Expose voicecall related enums to plugins

[Slava Monich]

On 25/06/18 18:40, Denis Kenzior wrote:

Hi Slava,

On 06/21/2018 12:20 PM, Slava Monich wrote:

---
  include/types.h | 31 +++
  1 file changed, 31 insertions(+)



Can you help me understand what is the motivation behind this?  In the 
past we opted not to expose OFONO enums for well-known values (e.g. 
those defined in 27.007).


Obviously I didn't know that it was a conscious decision. Feel free to 
ignore all those patches then. It's just I have very low tolerance to 
duplicate definitions and magic numbers.


common.h is not available to dynamically loadable plugins built against 
ofono devel package.


Also, enum ofono_clir_option (which seems to be a 27.007 thing too) is 
defined in include/types.h which is a bit inconsistent.


Oh well.

-S.
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] voicecall: Handle voicecall_dbus_register() return value

[Slava Monich]

FALSE means that struct voicecall passed in as a parameter
has been deallocated.
---
 src/voicecall.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index e4f6a4c..194b75f 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -1391,7 +1391,9 @@ static struct voicecall *synthesize_outgoing_call(struct 
ofono_voicecall *vc,
v->detect_time = time(NULL);
 
DBG("Registering new call: %d", call->id);
-   voicecall_dbus_register(v);
+
+   if (!voicecall_dbus_register(v))
+   return NULL;
 
vc->call_list = g_slist_insert_sorted(vc->call_list, v, call_compare);
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 11/12] voicecall: Use new voicecall enums

[Slava Monich]

---
 src/voicecall.c | 144 
 1 file changed, 73 insertions(+), 71 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index e4f6a4c..a5de038 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -177,10 +177,10 @@ static const char *disconnect_reason_to_string(enum 
ofono_disconnect_reason r)
 static const char *phone_and_clip_to_string(const struct ofono_phone_number *n,
int clip_validity)
 {
-   if (clip_validity == CLIP_VALIDITY_WITHHELD && !strlen(n->number))
+   if (clip_validity == OFONO_CLIP_VALIDITY_WITHHELD && !strlen(n->number))
return "withheld";
 
-   if (clip_validity == CLIP_VALIDITY_NOT_AVAILABLE)
+   if (clip_validity == OFONO_CLIP_VALIDITY_NOT_AVAILABLE)
return "";
 
return phone_number_to_string(n);
@@ -188,10 +188,10 @@ static const char *phone_and_clip_to_string(const struct 
ofono_phone_number *n,
 
 static const char *cnap_to_string(const char *name, int cnap_validity)
 {
-   if (cnap_validity == CNAP_VALIDITY_WITHHELD && !strlen(name))
+   if (cnap_validity == OFONO_CNAP_VALIDITY_WITHHELD && !strlen(name))
return "withheld";
 
-   if (cnap_validity == CNAP_VALIDITY_NOT_AVAILABLE)
+   if (cnap_validity == OFONO_CNAP_VALIDITY_NOT_AVAILABLE)
return "";
 
return name;
@@ -227,20 +227,20 @@ static unsigned int voicecalls_num_with_status(struct 
ofono_voicecall *vc,
 
 static unsigned int voicecalls_num_active(struct ofono_voicecall *vc)
 {
-   return voicecalls_num_with_status(vc, CALL_STATUS_ACTIVE);
+   return voicecalls_num_with_status(vc, OFONO_CALL_STATUS_ACTIVE);
 }
 
 static unsigned int voicecalls_num_held(struct ofono_voicecall *vc)
 {
-   return voicecalls_num_with_status(vc, CALL_STATUS_HELD);
+   return voicecalls_num_with_status(vc, OFONO_CALL_STATUS_HELD);
 }
 
 static unsigned int voicecalls_num_connecting(struct ofono_voicecall *vc)
 {
unsigned int r = 0;
 
-   r += voicecalls_num_with_status(vc, CALL_STATUS_DIALING);
-   r += voicecalls_num_with_status(vc, CALL_STATUS_ALERTING);
+   r += voicecalls_num_with_status(vc, OFONO_CALL_STATUS_DIALING);
+   r += voicecalls_num_with_status(vc, OFONO_CALL_STATUS_ALERTING);
 
return r;
 }
@@ -253,9 +253,9 @@ static gboolean voicecalls_have_active(struct 
ofono_voicecall *vc)
for (l = vc->call_list; l; l = l->next) {
v = l->data;
 
-   if (v->call->status == CALL_STATUS_ACTIVE ||
-   v->call->status == CALL_STATUS_DIALING ||
-   v->call->status == CALL_STATUS_ALERTING)
+   if (v->call->status == OFONO_CALL_STATUS_ACTIVE ||
+   v->call->status == OFONO_CALL_STATUS_DIALING ||
+   v->call->status == OFONO_CALL_STATUS_ALERTING)
return TRUE;
}
 
@@ -280,17 +280,17 @@ static gboolean voicecalls_have_with_status(struct 
ofono_voicecall *vc,
 
 static gboolean voicecalls_have_held(struct ofono_voicecall *vc)
 {
-   return voicecalls_have_with_status(vc, CALL_STATUS_HELD);
+   return voicecalls_have_with_status(vc, OFONO_CALL_STATUS_HELD);
 }
 
 static gboolean voicecalls_have_waiting(struct ofono_voicecall *vc)
 {
-   return voicecalls_have_with_status(vc, CALL_STATUS_WAITING);
+   return voicecalls_have_with_status(vc, OFONO_CALL_STATUS_WAITING);
 }
 
 static gboolean voicecalls_have_incoming(struct ofono_voicecall *vc)
 {
-   return voicecalls_have_with_status(vc, CALL_STATUS_INCOMING);
+   return voicecalls_have_with_status(vc, OFONO_CALL_STATUS_INCOMING);
 }
 
 static void dial_request_finish(struct ofono_voicecall *vc)
@@ -314,11 +314,11 @@ static gboolean voicecalls_can_dtmf(struct 
ofono_voicecall *vc)
for (l = vc->call_list; l; l = l->next) {
v = l->data;
 
-   if (v->call->status == CALL_STATUS_ACTIVE)
+   if (v->call->status == OFONO_CALL_STATUS_ACTIVE)
return TRUE;
 
/* Connected for 2nd stage dialing */
-   if (v->call->status == CALL_STATUS_ALERTING)
+   if (v->call->status == OFONO_CALL_STATUS_ALERTING)
return TRUE;
}
 
@@ -405,7 +405,7 @@ static void append_voicecall_properties(struct voicecall *v,
 
ofono_dbus_dict_append(dict, "State", DBUS_TYPE_STRING, );
 
-   if (call->direction == CALL_DIRECTION_MOBILE_TERMINATED)
+   if (call->direction == OFONO_CALL_DIRECTION_MOBILE_TERMINATED)
callerid = phone_and_clip_to_string(>phone_number,
call->clip_validity);
else
@@ -427,9 +427,9 @@ static void append_voicecall_properties(struct voicecall *v,
 
ofono_dbus_dict_append(dict, "Name", DBUS_TYPE_STRING, );
 

[PATCH 12/12] common: Use new voicecall enums

[Slava Monich]

---
 src/common.c | 20 ++--
 src/common.h | 33 +
 2 files changed, 11 insertions(+), 42 deletions(-)

diff --git a/src/common.c b/src/common.c
index 3ccaf7c..06b3521 100644
--- a/src/common.c
+++ b/src/common.c
@@ -740,26 +740,26 @@ const char *ofono_uuid_to_str(const struct ofono_uuid 
*uuid)
 void ofono_call_init(struct ofono_call *call)
 {
memset(call, 0, sizeof(struct ofono_call));
-   call->cnap_validity = CNAP_VALIDITY_NOT_AVAILABLE;
-   call->clip_validity = CLIP_VALIDITY_NOT_AVAILABLE;
+   call->cnap_validity = OFONO_CNAP_VALIDITY_NOT_AVAILABLE;
+   call->clip_validity = OFONO_CLIP_VALIDITY_NOT_AVAILABLE;
 }
 
-const char *call_status_to_string(enum call_status status)
+const char *call_status_to_string(enum ofono_call_status status)
 {
switch (status) {
-   case CALL_STATUS_ACTIVE:
+   case OFONO_CALL_STATUS_ACTIVE:
return "active";
-   case CALL_STATUS_HELD:
+   case OFONO_CALL_STATUS_HELD:
return "held";
-   case CALL_STATUS_DIALING:
+   case OFONO_CALL_STATUS_DIALING:
return "dialing";
-   case CALL_STATUS_ALERTING:
+   case OFONO_CALL_STATUS_ALERTING:
return "alerting";
-   case CALL_STATUS_INCOMING:
+   case OFONO_CALL_STATUS_INCOMING:
return "incoming";
-   case CALL_STATUS_WAITING:
+   case OFONO_CALL_STATUS_WAITING:
return "waiting";
-   case CALL_STATUS_DISCONNECTED:
+   case OFONO_CALL_STATUS_DISCONNECTED:
return "disconnected";
}
 
diff --git a/src/common.h b/src/common.h
index 1b6b01d..5fc9617 100644
--- a/src/common.h
+++ b/src/common.h
@@ -53,13 +53,6 @@ enum operator_status {
OPERATOR_STATUS_FORBIDDEN = 3,
 };
 
-/* 27.007 Section 7.6 */
-enum clip_validity {
-   CLIP_VALIDITY_VALID =   0,
-   CLIP_VALIDITY_WITHHELD =1,
-   CLIP_VALIDITY_NOT_AVAILABLE =   2,
-};
-
 /* 27.007 Section 7.29 */
 enum packet_bearer {
PACKET_BEARER_NONE =0,
@@ -72,30 +65,6 @@ enum packet_bearer {
PACKET_BEARER_EPS = 7,
 };
 
-/* 27.007 Section 7.30 */
-enum cnap_validity {
-   CNAP_VALIDITY_VALID =   0,
-   CNAP_VALIDITY_WITHHELD =1,
-   CNAP_VALIDITY_NOT_AVAILABLE =   2,
-};
-
-/* 27.007 Section 7.18 */
-enum call_status {
-   CALL_STATUS_ACTIVE =0,
-   CALL_STATUS_HELD =  1,
-   CALL_STATUS_DIALING =   2,
-   CALL_STATUS_ALERTING =  3,
-   CALL_STATUS_INCOMING =  4,
-   CALL_STATUS_WAITING =   5,
-   CALL_STATUS_DISCONNECTED
-};
-
-/* 27.007 Section 7.18 */
-enum call_direction {
-   CALL_DIRECTION_MOBILE_ORIGINATED =  0,
-   CALL_DIRECTION_MOBILE_TERMINATED =  1,
-};
-
 /* 27.007 Section 7.11 */
 enum bearer_class {
BEARER_CLASS_VOICE =1,
@@ -184,4 +153,4 @@ const char *registration_tech_to_string(int tech);
 const char *packet_bearer_to_string(int bearer);
 
 gboolean is_valid_apn(const char *apn);
-const char *call_status_to_string(enum call_status status);
+const char *call_status_to_string(enum ofono_call_status status);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 08/12] cdma-voicecall: Use new voicecall enums

[Slava Monich]

---
 src/cdma-voicecall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/cdma-voicecall.c b/src/cdma-voicecall.c
index fd38dd8..90cad10 100644
--- a/src/cdma-voicecall.c
+++ b/src/cdma-voicecall.c
@@ -251,7 +251,7 @@ static void manager_dial_callback(const struct ofono_error 
*error, void *data)
}
 
voicecall_set_call_lineid(vc);
-   vc->direction = CALL_DIRECTION_MOBILE_ORIGINATED;
+   vc->direction = OFONO_CALL_DIRECTION_MOBILE_ORIGINATED;
voicecall_set_call_status(vc, CDMA_CALL_STATUS_DIALING);
 
reply = dbus_message_new_method_return(vc->pending);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 09/12] emulator: Use new voicecall enums

[Slava Monich]

---
 src/emulator.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/emulator.c b/src/emulator.c
index ccb26dc..bda8f35 100644
--- a/src/emulator.c
+++ b/src/emulator.c
@@ -404,9 +404,9 @@ static gboolean notify_ccwa(void *user_data)
!em->ccwa)
goto end;
 
-   c = find_call_with_status(em, CALL_STATUS_WAITING);
+   c = find_call_with_status(em, OFONO_CALL_STATUS_WAITING);
 
-   if (c && c->clip_validity == CLIP_VALIDITY_VALID) {
+   if (c && c->clip_validity == OFONO_CLIP_VALIDITY_VALID) {
phone = phone_number_to_string(>phone_number);
sprintf(str, "+CCWA: \"%s\",%d", phone, c->phone_number.type);
 
@@ -439,19 +439,19 @@ static gboolean notify_ring(void *user_data)
if (!em->clip)
return TRUE;
 
-   c = find_call_with_status(em, CALL_STATUS_INCOMING);
+   c = find_call_with_status(em, OFONO_CALL_STATUS_INCOMING);
 
if (c == NULL)
return TRUE;
 
switch (c->clip_validity) {
-   case CLIP_VALIDITY_VALID:
+   case OFONO_CLIP_VALIDITY_VALID:
phone = phone_number_to_string(>phone_number);
sprintf(str, "+CLIP: \"%s\",%d", phone, c->phone_number.type);
g_at_server_send_unsolicited(em->server, str);
break;
 
-   case CLIP_VALIDITY_WITHHELD:
+   case OFONO_CLIP_VALIDITY_WITHHELD:
g_at_server_send_unsolicited(em->server, "+CLIP: \"\",128");
break;
}
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 10/12] stk: Use new voicecall enums

[Slava Monich]

---
 src/stk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 9cdf2b1..eb27555 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1732,7 +1732,7 @@ static void call_setup_connected(struct ofono_call *call, 
void *data)
static struct ofono_error error = { .type = OFONO_ERROR_TYPE_FAILURE };
static unsigned char facility_rejected_result[] = { 0x9d };
 
-   if (call == NULL || call->status == CALL_STATUS_DISCONNECTED) {
+   if (call == NULL || call->status == OFONO_CALL_STATUS_DISCONNECTED) {
memset(, 0, sizeof(rsp));
 
ADD_ERROR_RESULT(rsp.result,
@@ -1745,7 +1745,7 @@ static void call_setup_connected(struct ofono_call *call, 
void *data)
return;
}
 
-   if (call->status == CALL_STATUS_ACTIVE)
+   if (call->status == OFONO_CALL_STATUS_ACTIVE)
send_simple_response(stk, STK_RESULT_TYPE_SUCCESS);
else
send_simple_response(stk, STK_RESULT_TYPE_USER_CANCEL);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 06/12] rilmodem: Use new voicecall enums

[Slava Monich]

---
 drivers/rilmodem/voicecall.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rilmodem/voicecall.c b/drivers/rilmodem/voicecall.c
index b7180b9..585f216 100644
--- a/drivers/rilmodem/voicecall.c
+++ b/drivers/rilmodem/voicecall.c
@@ -288,7 +288,7 @@ no_calls:
 * arrives, or RING is used, then signal the call
 * here
 */
-   if (nc->status == CALL_STATUS_INCOMING &&
+   if (nc->status == OFONO_CALL_STATUS_INCOMING &&
(vd->flags & FLAG_NEED_CLIP)) {
if (nc->type)
ofono_voicecall_notify(vc, nc);
@@ -490,7 +490,7 @@ void ril_dial(struct ofono_voicecall *vc, const struct 
ofono_phone_number *ph,
for (l = vd->calls; l; l = l->next) {
call = l->data;
 
-   if (call->status == CALL_STATUS_ACTIVE) {
+   if (call->status == OFONO_CALL_STATUS_ACTIVE) {
current_active = 1;
break;
}
@@ -536,7 +536,7 @@ void ril_hangup_all(struct ofono_voicecall *vc, 
ofono_voicecall_cb_t cb,
for (l = vd->calls; l; l = l->next) {
call = l->data;
 
-   if (call->status == CALL_STATUS_INCOMING) {
+   if (call->status == OFONO_CALL_STATUS_INCOMING) {
/*
 * Need to use this request so that declined
 * calls in this state, are properly forwarded
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 03/12] hfpmodem: Use new voicecall enums

[Slava Monich]

---
 drivers/hfpmodem/voicecall.c | 92 +++-
 1 file changed, 48 insertions(+), 44 deletions(-)

diff --git a/drivers/hfpmodem/voicecall.c b/drivers/hfpmodem/voicecall.c
index 6ffe9d5..1ab2aa2 100644
--- a/drivers/hfpmodem/voicecall.c
+++ b/drivers/hfpmodem/voicecall.c
@@ -84,13 +84,14 @@ static GSList *find_dialing(GSList *calls)
 {
GSList *c;
 
-   c = g_slist_find_custom(calls, GINT_TO_POINTER(CALL_STATUS_DIALING),
+   c = g_slist_find_custom(calls,
+   GINT_TO_POINTER(OFONO_CALL_STATUS_DIALING),
at_util_call_compare_by_status);
 
if (c == NULL)
c = g_slist_find_custom(calls,
-   GINT_TO_POINTER(CALL_STATUS_ALERTING),
-   at_util_call_compare_by_status);
+   GINT_TO_POINTER(OFONO_CALL_STATUS_ALERTING),
+   at_util_call_compare_by_status);
 
return c;
 }
@@ -104,7 +105,8 @@ static void voicecall_notify(gpointer value, gpointer user)
 }
 
 static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
-   int direction, int status,
+   enum ofono_call_direction direction,
+   enum ofono_call_status status,
const char *num, int num_type, int clip)
 {
struct voicecall_data *d = ofono_voicecall_get_data(vc);
@@ -230,10 +232,10 @@ static void clcc_poll_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
nc = n ? n->data : NULL;
oc = o ? o->data : NULL;
 
-   if (nc && (nc->status == CALL_STATUS_ACTIVE))
+   if (nc && (nc->status == OFONO_CALL_STATUS_ACTIVE))
num_active++;
 
-   if (nc && (nc->status == CALL_STATUS_HELD))
+   if (nc && (nc->status == OFONO_CALL_STATUS_HELD))
num_held++;
 
if (oc && (nc == NULL || (nc->id > oc->id))) {
@@ -361,14 +363,15 @@ static void atd_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
for (l = vd->calls; l; l = l->next) {
call = l->data;
 
-   if (call->status != CALL_STATUS_ACTIVE)
+   if (call->status != OFONO_CALL_STATUS_ACTIVE)
continue;
 
-   call->status = CALL_STATUS_HELD;
+   call->status = OFONO_CALL_STATUS_HELD;
ofono_voicecall_notify(vc, call);
}
 
-   call = create_call(vc, 0, 0, CALL_STATUS_DIALING, NULL, type, validity);
+   call = create_call(vc, 0, OFONO_CALL_DIRECTION_MOBILE_ORIGINATED,
+   OFONO_CALL_STATUS_DIALING, NULL, type, validity);
if (call == NULL) {
ofono_error("Unable to allocate call, "
"call tracking will fail!");
@@ -478,10 +481,10 @@ static void hfp_answer(struct ofono_voicecall *vc,
 static void hfp_hangup(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
-   unsigned int affected = (1 << CALL_STATUS_INCOMING) |
-   (1 << CALL_STATUS_DIALING) |
-   (1 << CALL_STATUS_ALERTING) |
-   (1 << CALL_STATUS_ACTIVE);
+   unsigned int affected = (1 << OFONO_CALL_STATUS_INCOMING) |
+   (1 << OFONO_CALL_STATUS_DIALING) |
+   (1 << OFONO_CALL_STATUS_ALERTING) |
+   (1 << OFONO_CALL_STATUS_ACTIVE);
 
/* Hangup current active call */
hfp_template("AT+CHUP", vc, generic_cb, affected, cb, data);
@@ -504,7 +507,7 @@ static void hfp_release_all_held(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
struct voicecall_data *vd = ofono_voicecall_get_data(vc);
-   unsigned int held_status = 1 << CALL_STATUS_HELD;
+   unsigned int held_status = 1 << OFONO_CALL_STATUS_HELD;
 
if (vd->ag_mpty_features & HFP_AG_CHLD_0) {
hfp_template("AT+CHLD=0", vc, generic_cb, held_status,
@@ -519,7 +522,7 @@ static void hfp_set_udub(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
struct voicecall_data *vd = ofono_voicecall_get_data(vc);
-   unsigned int incoming_or_waiting = 1 << CALL_STATUS_WAITING;
+   unsigned int incoming_or_waiting = 1 << OFONO_CALL_STATUS_WAITING;
 
if (vd->ag_mpty_features & HFP_AG_CHLD_0) {
hfp_template("AT+CHLD=0", vc, generic_cb, incoming_or_waiting,
@@ -759,7 +762,7 @@ static void ccwa_notify(GAtResult *result, gpointer 
user_data)
 
/* CCWA can repeat, ignore if we already have an waiting call */
if (g_slist_find_custom(vd->calls,
-

[PATCH 01/12] include: Expose voicecall related enums to plugins

[Slava Monich]

---
 include/types.h | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/include/types.h b/include/types.h
index 2c64b20..a562bfe 100644
--- a/include/types.h
+++ b/include/types.h
@@ -49,6 +49,37 @@ enum ofono_clir_option {
OFONO_CLIR_OPTION_SUPPRESSION,
 };
 
+/* 27.007 Section 7.6 */
+enum ofono_clip_validity {
+   OFONO_CLIP_VALIDITY_VALID = 0,
+   OFONO_CLIP_VALIDITY_WITHHELD,
+   OFONO_CLIP_VALIDITY_NOT_AVAILABLE
+};
+
+/* 27.007 Section 7.18 */
+enum ofono_call_status {
+   OFONO_CALL_STATUS_ACTIVE = 0,
+   OFONO_CALL_STATUS_HELD,
+   OFONO_CALL_STATUS_DIALING,
+   OFONO_CALL_STATUS_ALERTING,
+   OFONO_CALL_STATUS_INCOMING,
+   OFONO_CALL_STATUS_WAITING,
+   OFONO_CALL_STATUS_DISCONNECTED
+};
+
+/* 27.007 Section 7.18 */
+enum ofono_call_direction {
+   OFONO_CALL_DIRECTION_MOBILE_ORIGINATED = 0,
+   OFONO_CALL_DIRECTION_MOBILE_TERMINATED
+};
+
+/* 27.007 Section 7.30 */
+enum ofono_cnap_validity {
+   OFONO_CNAP_VALIDITY_VALID = 0,
+   OFONO_CNAP_VALIDITY_WITHHELD,
+   OFONO_CNAP_VALIDITY_NOT_AVAILABLE
+};
+
 enum ofono_error_type {
OFONO_ERROR_TYPE_NO_ERROR = 0,
OFONO_ERROR_TYPE_CME,
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 04/12] huaweimodem: Use new voicecall enums

[Slava Monich]

---
 drivers/huaweimodem/voicecall.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/huaweimodem/voicecall.c b/drivers/huaweimodem/voicecall.c
index f55568d..36a16a1 100644
--- a/drivers/huaweimodem/voicecall.c
+++ b/drivers/huaweimodem/voicecall.c
@@ -49,7 +49,8 @@ struct voicecall_data {
 };
 
 static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
-   int direction, int status,
+   enum ofono_call_direction direction,
+   enum ofono_call_status status,
const char *num, int num_type,
int clip, int id)
 {
@@ -178,7 +179,7 @@ static void cring_notify(GAtResult *result, gpointer 
user_data)
 
/* CRING can repeat, ignore if we already have an incoming call */
if (g_slist_find_custom(vd->calls,
-   GINT_TO_POINTER(CALL_STATUS_INCOMING),
+   GINT_TO_POINTER(OFONO_CALL_STATUS_INCOMING),
at_util_call_compare_by_status))
return;
 
@@ -200,7 +201,8 @@ static void cring_notify(GAtResult *result, gpointer 
user_data)
id = ofono_voicecall_get_next_callid(vc);
 
/* Generate an incoming call */
-   create_call(vc, type, 1, CALL_STATUS_INCOMING, NULL, 128, 2, id);
+   create_call(vc, type, OFONO_CALL_DIRECTION_MOBILE_TERMINATED,
+   OFONO_CALL_STATUS_INCOMING, NULL, 128, 2, id);
 
/* Assume the CLIP always arrives, and we signal the call there */
DBG("%d", type);
@@ -217,7 +219,7 @@ static void clip_notify(GAtResult *result, gpointer 
user_data)
struct ofono_call *call;
 
l = g_slist_find_custom(vd->calls,
-   GINT_TO_POINTER(CALL_STATUS_INCOMING),
+   GINT_TO_POINTER(OFONO_CALL_STATUS_INCOMING),
at_util_call_compare_by_status);
if (l == NULL) {
ofono_error("CLIP for unknown call");
@@ -316,8 +318,9 @@ static void orig_notify(GAtResult *result, gpointer 
user_data)
 
ofono_info("Call origin: id %d type %d", call_id, call_type);
 
-   call = create_call(vc, call_type, 0, CALL_STATUS_DIALING, NULL, 128, 2,
-   call_id);
+   call = create_call(vc, call_type,
+   OFONO_CALL_DIRECTION_MOBILE_ORIGINATED,
+   OFONO_CALL_STATUS_DIALING, NULL, 128, 2, call_id);
if (call == NULL) {
ofono_error("Unable to malloc, call tracking will fail!");
return;
@@ -355,7 +358,7 @@ static void conf_notify(GAtResult *result, gpointer 
user_data)
 
/* Set call to alerting */
call = l->data;
-   call->status = CALL_STATUS_ALERTING;
+   call->status = OFONO_CALL_STATUS_ALERTING;
 
if (call->type == 0)
ofono_voicecall_notify(vc, call);
@@ -392,7 +395,7 @@ static void conn_notify(GAtResult *result, gpointer 
user_data)
 
/* Set call to active */
call = l->data;
-   call->status = CALL_STATUS_ACTIVE;
+   call->status = OFONO_CALL_STATUS_ACTIVE;
 
if (call->type == 0)
ofono_voicecall_notify(vc, call);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 07/12] stemodem: Use new voicecall enums

[Slava Monich]

---
 drivers/stemodem/voicecall.c | 77 +++-
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/drivers/stemodem/voicecall.c b/drivers/stemodem/voicecall.c
index 356ab7c..eb4431c 100644
--- a/drivers/stemodem/voicecall.c
+++ b/drivers/stemodem/voicecall.c
@@ -82,28 +82,29 @@ static int call_status_ste_to_ofono(enum call_status_ste 
status)
switch (status) {
case STE_CALL_STATUS_IDLE:
case STE_CALL_STATUS_RELEASED:
-   return CALL_STATUS_DISCONNECTED;
+   return OFONO_CALL_STATUS_DISCONNECTED;
case STE_CALL_STATUS_CALLING:
-   return CALL_STATUS_DIALING;
+   return OFONO_CALL_STATUS_DIALING;
case STE_CALL_STATUS_CONNECTING:
-   return CALL_STATUS_ALERTING;
+   return OFONO_CALL_STATUS_ALERTING;
case STE_CALL_STATUS_ACTIVE:
-   return CALL_STATUS_ACTIVE;
+   return OFONO_CALL_STATUS_ACTIVE;
case STE_CALL_STATUS_HOLD:
-   return CALL_STATUS_HELD;
+   return OFONO_CALL_STATUS_HELD;
case STE_CALL_STATUS_WAITING:
-   return CALL_STATUS_WAITING;
+   return OFONO_CALL_STATUS_WAITING;
case STE_CALL_STATUS_ALERTING:
-   return CALL_STATUS_INCOMING;
+   return OFONO_CALL_STATUS_INCOMING;
case STE_CALL_STATUS_BUSY:
-   return CALL_STATUS_DISCONNECTED;
+   return OFONO_CALL_STATUS_DISCONNECTED;
}
 
-   return CALL_STATUS_DISCONNECTED;
+   return OFONO_CALL_STATUS_DISCONNECTED;
 }
 
 static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
-   int direction, int status,
+   enum ofono_call_direction direction,
+   enum ofono_call_status status,
const char *num, int num_type, int clip)
 {
struct voicecall_data *d = ofono_voicecall_get_data(vc);
@@ -120,7 +121,7 @@ static struct ofono_call *create_call(struct 
ofono_voicecall *vc, int type,
call->direction = direction;
call->status = status;
 
-   if (clip != CLIP_VALIDITY_NOT_AVAILABLE) {
+   if (clip != OFONO_CLIP_VALIDITY_NOT_AVAILABLE) {
strncpy(call->phone_number.number, num,
OFONO_MAX_PHONE_NUMBER_LENGTH);
call->phone_number.type = num_type;
@@ -255,10 +256,10 @@ static void ste_hangup(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
unsigned int active_dial_alert_or_incoming =
-   (1 << CALL_STATUS_ACTIVE) |
-   (1 << CALL_STATUS_DIALING) |
-   (1 << CALL_STATUS_ALERTING) |
-   (1 << CALL_STATUS_INCOMING);
+   (1 << OFONO_CALL_STATUS_ACTIVE) |
+   (1 << OFONO_CALL_STATUS_DIALING) |
+   (1 << OFONO_CALL_STATUS_ALERTING) |
+   (1 << OFONO_CALL_STATUS_INCOMING);
 
ste_template("AT+CHUP", vc, ste_generic_cb,
active_dial_alert_or_incoming, cb, data);
@@ -273,7 +274,7 @@ static void ste_hold_all_active(struct ofono_voicecall *vc,
 static void ste_release_all_held(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
-   unsigned int held = 1 << CALL_STATUS_HELD;
+   unsigned int held = 1 << OFONO_CALL_STATUS_HELD;
 
ste_template("AT+CHLD=0", vc, ste_generic_cb, held, cb, data);
 }
@@ -282,7 +283,8 @@ static void ste_set_udub(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
unsigned int incoming_or_waiting =
-   (1 << CALL_STATUS_INCOMING) | (1 << 
CALL_STATUS_WAITING);
+   (1 << OFONO_CALL_STATUS_INCOMING) |
+   (1 << OFONO_CALL_STATUS_WAITING);
 
ste_template("AT+CHLD=0", vc, ste_generic_cb, incoming_or_waiting,
cb, data);
@@ -291,7 +293,7 @@ static void ste_set_udub(struct ofono_voicecall *vc,
 static void ste_release_all_active(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
-   unsigned int active = 1 << CALL_STATUS_ACTIVE;
+   unsigned int active = 1 << OFONO_CALL_STATUS_ACTIVE;
 
ste_template("AT+CHLD=1", vc, ste_generic_cb, active, cb, data);
 }
@@ -359,7 +361,8 @@ static void ste_deflect(struct ofono_voicecall *vc,
 {
char buf[128];
unsigned int incoming_or_waiting =
-   (1 << CALL_STATUS_INCOMING) | (1 << CALL_STATUS_WAITING);
+   (1 << OFONO_CALL_STATUS_INCOMING) |
+   (1 << OFONO_CALL_STATUS_WAITING);
 
snprintf(buf, sizeof(buf), "AT+CTFR=\"%s\",%d", ph->number, ph->type);
ste_template(buf, vc, 

[PATCH 05/12] ifxmodem: Use new voicecall enums

[Slava Monich]

---
 drivers/ifxmodem/voicecall.c | 59 
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/drivers/ifxmodem/voicecall.c b/drivers/ifxmodem/voicecall.c
index 45b5ca4..9055b2d 100644
--- a/drivers/ifxmodem/voicecall.c
+++ b/drivers/ifxmodem/voicecall.c
@@ -80,7 +80,8 @@ static int class_to_call_type(int cls)
 }
 
 static struct ofono_call *create_call(struct ofono_voicecall *vc, int type,
-   int direction, int status,
+   enum ofono_call_direction direction,
+   enum ofono_call_status status,
const char *num, int num_type,
int clip, int id)
 {
@@ -137,9 +138,9 @@ static void xcallstat_notify(GAtResult *result, gpointer 
user_data)
l = g_slist_find_custom(vd->calls, GINT_TO_POINTER(id),
at_util_call_compare_by_id);
 
-   if (l == NULL && status != CALL_STATUS_DIALING &&
-   status != CALL_STATUS_INCOMING &&
-   status != CALL_STATUS_WAITING) {
+   if (l == NULL && status != OFONO_CALL_STATUS_DIALING &&
+   status != OFONO_CALL_STATUS_INCOMING &&
+   status != OFONO_CALL_STATUS_WAITING) {
ofono_error("Received XCALLSTAT for an untracked"
" call, this indicates a bug!");
return;
@@ -149,7 +150,7 @@ static void xcallstat_notify(GAtResult *result, gpointer 
user_data)
existing_call = l->data;
 
switch (status) {
-   case CALL_STATUS_DISCONNECTED:
+   case OFONO_CALL_STATUS_DISCONNECTED:
{
enum ofono_disconnect_reason reason;
 
@@ -168,10 +169,11 @@ static void xcallstat_notify(GAtResult *result, gpointer 
user_data)
g_free(existing_call);
break;
}
-   case CALL_STATUS_DIALING:
-   new_call = create_call(vc, 0, CALL_DIRECTION_MOBILE_ORIGINATED,
+   case OFONO_CALL_STATUS_DIALING:
+   new_call = create_call(vc, 0,
+   OFONO_CALL_DIRECTION_MOBILE_ORIGINATED,
status, NULL, 128,
-   CLIP_VALIDITY_NOT_AVAILABLE, id);
+   OFONO_CLIP_VALIDITY_NOT_AVAILABLE, id);
if (new_call == NULL) {
ofono_error("Unable to malloc. "
"Call management is fubar");
@@ -180,8 +182,8 @@ static void xcallstat_notify(GAtResult *result, gpointer 
user_data)
 
ofono_voicecall_notify(vc, new_call);
break;
-   case CALL_STATUS_WAITING:
-   case CALL_STATUS_INCOMING:
+   case OFONO_CALL_STATUS_WAITING:
+   case OFONO_CALL_STATUS_INCOMING:
/* Handle the following situation:
 * Active Call + Waiting Call. Active Call is Released.
 * The Waiting call becomes Incoming. In this case, no
@@ -193,9 +195,10 @@ static void xcallstat_notify(GAtResult *result, gpointer 
user_data)
return;
}
 
-   new_call = create_call(vc, 0, CALL_DIRECTION_MOBILE_TERMINATED,
+   new_call = create_call(vc, 0,
+   OFONO_CALL_DIRECTION_MOBILE_TERMINATED,
status, NULL, 128,
-   CLIP_VALIDITY_NOT_AVAILABLE, id);
+   OFONO_CLIP_VALIDITY_NOT_AVAILABLE, id);
if (new_call == NULL) {
ofono_error("Unable to malloc. "
"Call management is fubar");
@@ -203,13 +206,13 @@ static void xcallstat_notify(GAtResult *result, gpointer 
user_data)
}
 
break;
-   case CALL_STATUS_ALERTING:
-   case CALL_STATUS_ACTIVE:
-   case CALL_STATUS_HELD:
+   case OFONO_CALL_STATUS_ALERTING:
+   case OFONO_CALL_STATUS_ACTIVE:
+   case OFONO_CALL_STATUS_HELD:
default:
/* For connected status, simply reset back to active */
if (status == 7)
-   status = CALL_STATUS_ACTIVE;
+   status = OFONO_CALL_STATUS_ACTIVE;
 
existing_call->status = status;
ofono_voicecall_notify(vc, existing_call);
@@ -388,7 +391,7 @@ static void ifx_hold_all_active(struct ofono_voicecall *vc,
 static void ifx_release_all_held(struct ofono_voicecall *vc,
ofono_voicecall_cb_t cb, void *data)
 {
-   unsigned int held_status = 1 << CALL_STATUS_HELD;
+   unsigned int held_status = 1 << OFONO_CALL_STATUS_HELD;
ifx_template("AT+CHLD=0", vc, 

[PATCH 1/2] include: Add ofono_voicecall_get_modem

[Slava Monich]

---
 include/voicecall.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/voicecall.h b/include/voicecall.h
index 5b3da6a..a704b65 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -160,6 +160,8 @@ void ofono_voicecall_disconnected(struct ofono_voicecall 
*vc, int id,
  */
 void ofono_voicecall_mpty_hint(struct ofono_voicecall *vc, unsigned int ids);
 
+struct ofono_modem *ofono_voicecall_get_modem(struct ofono_voicecall *vc);
+
 int ofono_voicecall_driver_register(const struct ofono_voicecall_driver *d);
 void ofono_voicecall_driver_unregister(const struct ofono_voicecall_driver *d);
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 2/2] voicecall: Implement ofono_voicecall_get_modem

[Slava Monich]

---
 src/voicecall.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/voicecall.c b/src/voicecall.c
index e4f6a4c..7d4f6a1 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -3734,6 +3734,11 @@ void *ofono_voicecall_get_data(struct ofono_voicecall 
*vc)
return vc->driver_data;
 }
 
+struct ofono_modem *ofono_voicecall_get_modem(struct ofono_voicecall *vc)
+{
+   return __ofono_atom_get_modem(vc->atom);
+}
+
 int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc)
 {
struct ofono_modem *modem;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] ussd: Cancel pending requests when unregistering

[Slava Monich]

And reset state to idle before unregistering the D-Bus interface.
This may occur e.g. when we receive REFRESH from STK.
---
 src/ussd.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/src/ussd.c b/src/ussd.c
index f5dd9d9..23b3323 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -810,6 +810,22 @@ static void ussd_unregister(struct ofono_atom *atom)
DBusConnection *conn = ofono_dbus_get_connection();
struct ofono_modem *modem = __ofono_atom_get_modem(atom);
const char *path = __ofono_atom_get_path(atom);
+   DBusMessage *reply;
+
+   if (ussd->pending) {
+   reply = __ofono_error_canceled(ussd->pending);
+   __ofono_dbus_pending_reply(>pending, reply);
+   }
+
+   if (ussd->cancel) {
+   reply = dbus_message_new_method_return(ussd->cancel);
+   __ofono_dbus_pending_reply(>cancel, reply);
+   }
+
+   if (ussd->req)
+   ussd_request_finish(ussd, -ECANCELED, 0, NULL, 0);
+
+   ussd_change_state(ussd, USSD_STATE_IDLE);
 
g_slist_free_full(ussd->ss_control_list, ssc_entry_destroy);
ussd->ss_control_list = NULL;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 2/2] modem: Implement ofono_modem_get_gprs

[Slava Monich]

---
 src/modem.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/modem.c b/src/modem.c
index 0cee861..9409347 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -189,6 +189,11 @@ struct ofono_sim *ofono_modem_get_sim(struct ofono_modem 
*modem)
return __ofono_atom_find(OFONO_ATOM_TYPE_SIM, modem);
 }
 
+struct ofono_gprs *ofono_modem_get_gprs(struct ofono_modem *modem)
+{
+   return __ofono_atom_find(OFONO_ATOM_TYPE_GPRS, modem);
+}
+
 struct ofono_atom *__ofono_modem_add_atom(struct ofono_modem *modem,
enum ofono_atom_type type,
void (*destruct)(struct ofono_atom *),
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 1/2] include: Add ofono_modem_get_gprs

[Slava Monich]

---
 include/modem.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/modem.h b/include/modem.h
index c93b3d2..005a42e 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -29,6 +29,7 @@ extern "C" {
 #include 
 
 struct ofono_modem;
+struct ofono_gprs;
 struct ofono_sim;
 
 enum ofono_modem_type {
@@ -82,6 +83,7 @@ void ofono_modem_remove_interface(struct ofono_modem *modem,
 
 const char *ofono_modem_get_path(struct ofono_modem *modem);
 struct ofono_sim *ofono_modem_get_sim(struct ofono_modem *modem);
+struct ofono_gprs *ofono_modem_get_gprs(struct ofono_modem *modem);
 
 void ofono_modem_set_data(struct ofono_modem *modem, void *data);
 void *ofono_modem_get_data(struct ofono_modem *modem);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] ussd: Don't ignore data from TERMINATED response

[Slava Monich]

Typically responses to USSD requests are coming with status
zero (NOTIFY) but some are coming with status 2 (TERMINATED).
If those contain data, the data should be presented to the user.
---
 src/ussd.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/src/ussd.c b/src/ussd.c
index 84f64c6..f5dd9d9 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -417,13 +417,18 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int 
status, int dcs,
}
 
if (status == OFONO_USSD_STATUS_TERMINATED) {
-   ussd_change_state(ussd, USSD_STATE_IDLE);
+   if (ussd->state == USSD_STATE_ACTIVE && data && data_len > 0) {
+   /* Interpret that as a Notify */
+   status = OFONO_USSD_STATUS_NOTIFY;
+   } else {
+   ussd_change_state(ussd, USSD_STATE_IDLE);
 
-   if (ussd->pending == NULL)
-   return;
+   if (ussd->pending == NULL)
+   return;
 
-   reply = __ofono_error_network_terminated(ussd->pending);
-   goto out;
+   reply = __ofono_error_network_terminated(ussd->pending);
+   goto out;
+   }
}
 
if (status == OFONO_USSD_STATUS_NOT_SUPPORTED) {
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] phonebook: Fixed double deletion of merge_list

[Slava Monich]

---
 src/phonebook.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/phonebook.c b/src/phonebook.c
index 1e4efa2..f8936c5 100644
--- a/src/phonebook.c
+++ b/src/phonebook.c
@@ -428,7 +428,6 @@ static void export_phonebook_cb(const struct ofono_error 
*error, void *data)
g_slist_foreach(phonebook->merge_list, print_merged_entry,
phonebook->vcards);
g_slist_free_full(phonebook->merge_list, destroy_merged_entry);
-   g_slist_free(phonebook->merge_list);
phonebook->merge_list = NULL;
 
phonebook->storage_index++;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: Plugin infrastructure

[Slava Monich]

On 05/02/18 20:55, Jonas Bonn wrote:


On 02/05/2018 05:25 PM, Slava Monich wrote:

Hi Jonas,

On 02/04/2018 11:19 AM, Jonas Bonn wrote:

Hi,

I was reading through the ofono code and I see that there's a lot 
of code in place to support external plugins.  As things currently 
stand, however, ofono can only be built with _builtin_ modules, so 
all this plugin infrastructure is totally unused. So I had a couple 
of questions about this:


i)  Is there any value in keeping the plugin infrastructure in 
place? ...or would an effort to remove it be valued?


We did (still do?) have external plugins in use...


Yes, we do!


OK.  If you don't mind a stupid question:  why?  Why not just put 
these in the ofono tree?


Those are of no interest to the general public and not even compatible 
with the upstream - our fork is slightly different.


We wanted those to only be installed (pulled in via rpm dependencies) if 
the corresponding functionality is installed on the phone and 
dynamically loadable plugins fit quite nicely into the picture.


-Slava

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: Plugin infrastructure

[Slava Monich]

Hi Jonas,

On 02/04/2018 11:19 AM, Jonas Bonn wrote:

Hi,

I was reading through the ofono code and I see that there's a lot of 
code in place to support external plugins.  As things currently 
stand, however, ofono can only be built with _builtin_ modules, so 
all this plugin infrastructure is totally unused. So I had a couple 
of questions about this:


i)  Is there any value in keeping the plugin infrastructure in place? 
...or would an effort to remove it be valued?


We did (still do?) have external plugins in use...


Yes, we do!

If upstream eliminates the plugin system, we would still have to 
maintain it in our fork. At the moment we are using it more or less as is.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] unit: Improve idmap.c unit test coverage

[Slava Monich]

This brings function, line and branch coverage for idmap.c to 100%
---
 unit/test-idmap.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/unit/test-idmap.c b/unit/test-idmap.c
index b072933..2d2e226 100644
--- a/unit/test-idmap.c
+++ b/unit/test-idmap.c
@@ -35,9 +35,12 @@ static void test_alloc(void)
idmap = idmap_new(2);
 
g_assert(idmap);
+   g_assert(idmap_get_min(idmap) == 1);
 
bit = idmap_alloc(idmap);
g_assert(bit == 1);
+   g_assert(idmap_find(idmap, bit));
+   g_assert(!idmap_find(idmap, idmap_get_max(idmap) + 1));
 
bit = idmap_alloc(idmap);
g_assert(bit == 2);
@@ -62,6 +65,12 @@ static void test_alloc(void)
bit = idmap_alloc(idmap);
g_assert(bit == 1);
 
+   idmap_put(idmap, 1);
+   idmap_take(idmap, 1);
+   idmap_take(idmap, 3);
+   bit = idmap_alloc(idmap);
+   g_assert(bit == 2);
+
idmap_free(idmap);
 }
 
@@ -80,9 +89,24 @@ static void test_alloc_next(void)
bit = idmap_alloc_next(idmap, 255);
g_assert(bit == 1);
 
+   while (idmap_alloc(idmap) < (sizeof(unsigned long) * 8) + 1);
+   bit = idmap_alloc_next(idmap, 1);
+   g_assert(bit == (sizeof(unsigned long) * 8) + 2);
+
+   idmap_free(idmap);
+
+   idmap = idmap_new(2);
+
+   g_assert(idmap);
+   g_assert(idmap_alloc_next(idmap, 0) == 3);
+   g_assert(idmap_alloc_next(idmap, 3) == 3);
+
bit = idmap_alloc_next(idmap, 1);
g_assert(bit == 2);
 
+   bit = idmap_alloc_next(idmap, 2);
+   g_assert(bit == 1);
+
idmap_free(idmap);
 }
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] dbus: Use dbus_validate_path

[Slava Monich]

Instead of __ofono_dbus_valid_object_path
---
 plugins/push-notification.c |  2 +-
 plugins/smart-messaging.c   |  2 +-
 src/dbus.c  | 44 
 src/gnss.c  |  2 +-
 src/modem.c |  2 +-
 src/netmon.c|  2 +-
 src/ofono.h |  2 --
 src/stk.c   |  4 ++--
 8 files changed, 7 insertions(+), 53 deletions(-)

diff --git a/plugins/push-notification.c b/plugins/push-notification.c
index ff388d9..f469f9e 100644
--- a/plugins/push-notification.c
+++ b/plugins/push-notification.c
@@ -96,7 +96,7 @@ static DBusMessage 
*push_notification_register_agent(DBusConnection *conn,
DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
 
-   if (!__ofono_dbus_valid_object_path(agent_path))
+   if (!dbus_validate_path(agent_path, NULL))
return __ofono_error_invalid_format(msg);
 
pn->agent = sms_agent_new(AGENT_INTERFACE,
diff --git a/plugins/smart-messaging.c b/plugins/smart-messaging.c
index bbbdaa9..0c9700d 100644
--- a/plugins/smart-messaging.c
+++ b/plugins/smart-messaging.c
@@ -119,7 +119,7 @@ static DBusMessage 
*smart_messaging_register_agent(DBusConnection *conn,
DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
 
-   if (!__ofono_dbus_valid_object_path(agent_path))
+   if (!dbus_validate_path(agent_path, NULL))
return __ofono_error_invalid_format(msg);
 
sm->agent = sms_agent_new(AGENT_INTERFACE,
diff --git a/src/dbus.c b/src/dbus.c
index 45becc1..3e1e162 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -456,50 +456,6 @@ void __ofono_dbus_pending_reply(DBusMessage **msg, 
DBusMessage *reply)
*msg = NULL;
 }
 
-gboolean __ofono_dbus_valid_object_path(const char *path)
-{
-   unsigned int i;
-   char c = '\0';
-
-   if (path == NULL)
-   return FALSE;
-
-   if (path[0] == '\0')
-   return FALSE;
-
-   if (path[0] && !path[1] && path[0] == '/')
-   return TRUE;
-
-   if (path[0] != '/')
-   return FALSE;
-
-   for (i = 0; path[i]; i++) {
-   if (path[i] == '/' && c == '/')
-   return FALSE;
-
-   c = path[i];
-
-   if (path[i] >= 'a' && path[i] <= 'z')
-   continue;
-
-   if (path[i] >= 'A' && path[i] <= 'Z')
-   continue;
-
-   if (path[i] >= '0' && path[i] <= '9')
-   continue;
-
-   if (path[i] == '_' || path[i] == '/')
-   continue;
-
-   return FALSE;
-   }
-
-   if (path[i-1] == '/')
-   return FALSE;
-
-   return TRUE;
-}
-
 DBusConnection *ofono_dbus_get_connection(void)
 {
return g_connection;
diff --git a/src/gnss.c b/src/gnss.c
index 97d1152..ba2a97b 100644
--- a/src/gnss.c
+++ b/src/gnss.c
@@ -135,7 +135,7 @@ static DBusMessage *gnss_register_agent(DBusConnection 
*conn,
_path, DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
 
-   if (!__ofono_dbus_valid_object_path(agent_path))
+   if (!dbus_validate_path(agent_path, NULL))
return __ofono_error_invalid_format(msg);
 
gnss->posr_agent = gnss_agent_new(agent_path,
diff --git a/src/modem.c b/src/modem.c
index 0c63d2c..d5fda7c 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -1876,7 +1876,7 @@ struct ofono_modem *ofono_modem_create(const char *name, 
const char *type)
else
snprintf(path, sizeof(path), "/%s", name);
 
-   if (__ofono_dbus_valid_object_path(path) == FALSE)
+   if (!dbus_validate_path(path, NULL))
return NULL;
 
modem = g_try_new0(struct ofono_modem, 1);
diff --git a/src/netmon.c b/src/netmon.c
index 3a49587..62e0ec0 100644
--- a/src/netmon.c
+++ b/src/netmon.c
@@ -353,7 +353,7 @@ static DBusMessage *netmon_register_agent(DBusConnection 
*conn,
DBUS_TYPE_INVALID) == FALSE)
return __ofono_error_invalid_args(msg);
 
-   if (!__ofono_dbus_valid_object_path(agent_path))
+   if (!dbus_validate_path(agent_path, NULL))
return __ofono_error_invalid_format(msg);
 
if (!period)
diff --git a/src/ofono.h b/src/ofono.h
index deb1b7c..ac3726d 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -76,8 +76,6 @@ DBusMessage *__ofono_error_from_error(const struct 
ofono_error *error,
 
 void __ofono_dbus_pending_reply(DBusMessage **msg, DBusMessage *reply);
 
-gboolean __ofono_dbus_valid_object_path(const char *path);
-
 struct ofono_watchlist_item {
unsigned int id;
void *notify;
diff --git a/src/stk.c b/src/stk.c
index 49d6365..9cdf2b1 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -722,7 +722,7 @@ static 

Re: Problem with dialstring ##21#

[Slava Monich]

On 16/01/18 14:50, Engelbert Torremans wrote:

All,

I am looking for some help in trying to fix this problem I am 
experiencing when using Ofono on a RaspberryPi in "carkit/HFP mode".


Currently I can make calls (originate and terminate) and audio is fine 
in combination with Bluez5 etc. also.


But I cannot disable CallForwarding via the dial-number command.

Enabling CFU via **21*destination-number# is working fine.

Trying to disable CFU via ##21# gives below error. Doing the same via 
the keypad on the mobile phone works fine.


Are you sure that the number is quoted correctly so that the hash sign 
is not interpreted by your shell as a comment? Check with dbus-monitor 
what's actually being sent over D-Bus.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Don't submit parallel EFpl reads

[Slava Monich]

In addition to not doing unnecessary SIM I/O, this fixes memory leaks
like this one:

==10096== 74 (56 direct, 18 indirect) bytes in 2 blocks are definitely lost in 
loss record 1,252 of 1,342
==10096==at 0x4841BF0: calloc (vg_replace_malloc.c)
==10096==by 0x4B03117: g_malloc0 (gmem.c)
==10096==by 0xF83DF: concat_lang_prefs (sim.c)
==10096==by 0xF8697: sim_efpl_read_cb (sim.c)
==10096==by 0x12CBF7: sim_fs_op_read_block_cb (simfs.c)
---
 src/sim.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/sim.c b/src/sim.c
index 2538c77..31df636 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2200,6 +2200,9 @@ static void sim_efli_efpl_changed(int id, void *userdata)
if (sim->efli != NULL) /* This shouldn't happen */
return;
 
+   if (sim->language_prefs_update)
+   return;
+
if (sim->language_prefs) {
g_strfreev(sim->language_prefs);
sim->language_prefs = NULL;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] sim: Handle multiple EFpl read completions

[Slava Monich]

On 08/12/17 17:14, Denis Kenzior wrote:

...

Can we do something like
if (sim->language_prefs_update)
return;

?



Yes, of course! Forget this patch then, I'll send a new one.

Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] sim: Handle multiple EFpl read completions

[Slava Monich]

Since sim_efli_efpl_changed() can be invoked several times before
completion of SIM reads (which are not cancellable), followed by
multiple completions, it's not enough to free old sim->language_prefs
in sim_efli_efpl_changed(). The same thing has to be done by
sim_efpl_read_cb() to avoid leaking memory e.g. after this
sequence of calls:

...


There are more leaks like this in sim.c, you can expect a few more 
patches if this one gets applied.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Handle multiple EFpl read completions

[Slava Monich]

Since sim_efli_efpl_changed() can be invoked several times before
completion of SIM reads (which are not cancellable), followed by
multiple completions, it's not enough to free old sim->language_prefs
in sim_efli_efpl_changed(). The same thing has to be done by
sim_efpl_read_cb() to avoid leaking memory e.g. after this
sequence of calls:

  1. sim_efli_efpl_changed()
  2. sim_efli_efpl_changed()
  3. sim_efpl_read_cb()
  4. sim_efpl_read_cb()

One such scenario involves __ofono_sim_refresh() calling
sim_efli_efpl_changed() first with id=SIM_EFPL_FILEID,
like this:

  #0  sim_efli_efpl_changed (id=12037)
  #1  sim_fs_notify_file_watches (id=-1)
  #2  __ofono_sim_refresh (file_list=0x0, full_file_change=1, naa_init=1)
  #3  handle_command_refresh ()
  #4  ofono_stk_proactive_command_handled_notify ()
  ...

and then with id=28421 (SIM_EFLI_FILEID), so both files are
re-read twice, the second result overwriting the first one
and the first one getting lost (leaked). Since SIM reads are
asynchronous and take considerable time, I'm quite sure that
there are other scenarios as well.

In fact it's even better to keep the old sim->language_prefs
around until EFpl read is finished, to avoid issuing unnecessary
PropertyChanged signal if the second read produces the same
result, which it normally does.
---
 src/sim.c | 30 --
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 2538c77..2585f06 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2081,6 +2081,26 @@ static char **concat_lang_prefs(GSList *a, GSList *b)
return ret;
 }
 
+static gboolean strv_equal(char **sv1, char **sv2)
+{
+   const guint len1 = sv1 ? g_strv_length(sv1) : 0;
+   const guint len2 = sv2 ? g_strv_length(sv2) : 0;
+
+   if (len1 == len2) {
+   guint i;
+
+   for (i = 0; i < len1; i++) {
+   if (strcmp(sv1[i], sv2[i])) {
+   return FALSE;
+   }
+   }
+
+   return TRUE;
+   }
+
+   return FALSE;
+}
+
 static void sim_efpl_read_cb(int ok, int length, int record,
const unsigned char *data,
int record_length, void *userdata)
@@ -2091,6 +2111,7 @@ static void sim_efpl_read_cb(int ok, int length, int 
record,
gboolean efli_format = TRUE;
GSList *efli = NULL;
GSList *efpl = NULL;
+   char **old_prefs = sim->language_prefs;
 
if (!ok || length < 2)
goto skip_efpl;
@@ -2142,13 +2163,15 @@ skip_efpl:
g_slist_free_full(efpl, g_free);
}
 
-   if (sim->language_prefs != NULL)
+   if (!strv_equal(sim->language_prefs, old_prefs))
ofono_dbus_signal_array_property_changed(conn, path,
OFONO_SIM_MANAGER_INTERFACE,
"PreferredLanguages",
DBUS_TYPE_STRING,
>language_prefs);
 
+   g_strfreev(old_prefs);
+
/* Proceed with sim initialization if we're not merely updating */
if (!sim->language_prefs_update)
__ofono_sim_recheck_pin(sim);
@@ -2200,11 +2223,6 @@ static void sim_efli_efpl_changed(int id, void *userdata)
if (sim->efli != NULL) /* This shouldn't happen */
return;
 
-   if (sim->language_prefs) {
-   g_strfreev(sim->language_prefs);
-   sim->language_prefs = NULL;
-   }
-
sim->language_prefs_update = true;
 
ofono_sim_read(sim->early_context, SIM_EFLI_FILEID,
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim-auth: Avoid using dbus_message_iter_get_element_count

[Slava Monich]

It's the only thing in ofono that requires dbus 1.9.16 or later and it's
not worth it.

And don't leak DBusMessage on format error.
---
 src/sim-auth.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/src/sim-auth.c b/src/sim-auth.c
index f9f74d4..7b65738 100644
--- a/src/sim-auth.c
+++ b/src/sim-auth.c
@@ -369,9 +369,7 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection 
*conn,
struct ofono_sim_auth *sa = data;
DBusMessageIter iter;
DBusMessageIter array;
-   int i;
uint8_t *aid;
-   int rands;
 
if (sa->pending)
return __ofono_error_busy(msg);
@@ -381,27 +379,22 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection 
*conn,
if (dbus_message_iter_get_arg_type() != DBUS_TYPE_ARRAY)
return __ofono_error_invalid_format(msg);
 
-   rands = dbus_message_iter_get_element_count();
-
-   if (rands > 3 || rands < 2)
-   return __ofono_error_invalid_format(msg);
-
sa->pending = g_new0(struct auth_request, 1);
-   sa->pending->msg = dbus_message_ref(msg);
-   sa->pending->num_rands = rands;
 
dbus_message_iter_recurse(, );
 
-   for (i = 0; i < sa->pending->num_rands; i++) {
+   while (dbus_message_iter_get_arg_type() == DBUS_TYPE_ARRAY) {
int nelement;
DBusMessageIter in;
 
dbus_message_iter_recurse(, );
 
-   if (dbus_message_iter_get_arg_type() != DBUS_TYPE_BYTE)
+   if (dbus_message_iter_get_arg_type() != DBUS_TYPE_BYTE ||
+   sa->pending->num_rands == SIM_AUTH_MAX_RANDS)
goto format_error;
 
-   dbus_message_iter_get_fixed_array(, >pending->rands[i],
+   dbus_message_iter_get_fixed_array(,
+   >pending->rands[sa->pending->num_rands++],
);
 
if (nelement != 16)
@@ -410,12 +403,15 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection 
*conn,
dbus_message_iter_next();
}
 
+   if (sa->pending->num_rands < 2)
+   goto format_error;
+
/*
 * retrieve session from SIM
 */
aid = find_aid_by_path(sa->aid_objects, dbus_message_get_path(msg));
sa->pending->session = __ofono_sim_get_session_by_aid(sa->sim, aid);
-
+   sa->pending->msg = dbus_message_ref(msg);
sa->pending->watch_id = __ofono_sim_add_session_watch(
sa->pending->session, get_session_cb, sa, NULL);
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 2/2] storage: Implement ofono_config_dir and ofono_storage_dir

[Slava Monich]

---
 src/storage.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/src/storage.c b/src/storage.c
index bde0bea..9b7bfbc 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -23,6 +23,8 @@
 #include 
 #endif
 
+#include 
+
 #define _GNU_SOURCE
 #include 
 #include 
@@ -35,6 +37,16 @@
 
 #include "storage.h"
 
+const char *ofono_config_dir(void)
+{
+   return CONFIGDIR;
+}
+
+const char *ofono_storage_dir(void)
+{
+   return STORAGEDIR;
+}
+
 int create_dirs(const char *filename, const mode_t mode)
 {
struct stat st;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 1/2] include: Add storage.h

[Slava Monich]

To expose ofono directories to dynamically loadable plugins.
---
 Makefile.am   |  3 ++-
 include/storage.h | 32 
 2 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 include/storage.h

diff --git a/Makefile.am b/Makefile.am
index 903630b..f58cc92 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -22,7 +22,8 @@ pkginclude_HEADERS = include/log.h include/plugin.h 
include/history.h \
include/private-network.h include/cdma-netreg.h \
include/cdma-provision.h include/handsfree.h \
include/handsfree-audio.h include/siri.h \
-   include/netmon.h include/lte.h include/ims.h
+   include/netmon.h include/lte.h include/ims.h \
+   include/storage.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
diff --git a/include/storage.h b/include/storage.h
new file mode 100644
index 000..243eb88
--- /dev/null
+++ b/include/storage.h
@@ -0,0 +1,32 @@
+/*
+ *
+ *  oFono - Open Telephony stack for Linux
+ *
+ *  Copyright (C) 2017  Jolla Ltd. 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.
+ *
+ */
+
+#ifndef __OFONO_STORAGE_H
+#define __OFONO_STORAGE_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+const char *ofono_config_dir(void);
+const char *ofono_storage_dir(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_STORAGE_H */
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] sim: Move atom registration to the end of ofono_sim_register

[Slava Monich]

Hi Denis,

On 27/10/17 16:56, Denis Kenzior wrote:

Hi Slava,

On 10/27/2017 08:15 AM, Slava Monich wrote:

The state needs to be checked prior to calling __ofono_atom_register
because atom registration calls OFONO_ATOM_WATCH_CONDITION_REGISTERED
callbacks each of which may call ofono_sim_inserted_notify. Should
that happen, by the time __ofono_atom_register returns, ofono_sim
will be in OFONO_SIM_STATE_INSERTED state and sim_initialize will
be called twice if the initial state was OFONO_SIM_STATE_NOT_PRESENT.


Okay, can you tell me why you would have such a setup? sim_inserted is 
generally either called from the modem driver or when an unsolicited 
notification occurs.  Not when the sim atom is registered.




For the reasons too long to explain, I already know the SIM state by the 
time I register the SIM atom (or even the modem). I could work around 
this problem by delaying ofono_sim_inserted_notify call or partially 
duplicating the logic in my modem driver, but if the core can handle my 
setup, that's even better.


Thanks,

-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Move atom registration to the end of ofono_sim_register

[Slava Monich]

The state needs to be checked prior to calling __ofono_atom_register
because atom registration calls OFONO_ATOM_WATCH_CONDITION_REGISTERED
callbacks each of which may call ofono_sim_inserted_notify. Should
that happen, by the time __ofono_atom_register returns, ofono_sim
will be in OFONO_SIM_STATE_INSERTED state and sim_initialize will
be called twice if the initial state was OFONO_SIM_STATE_NOT_PRESENT.
If nothing else, that results in memory leaks like this one (because
IMSI will be queried twice, among other things):

==3017== 16 bytes in 1 blocks are definitely lost in loss record 187 of 475
==3017==at 0x483F380: malloc (vg_replace_malloc.c:296)
==3017==by 0x4AFB0DF: g_malloc (gmem.c:94)
==3017==by 0x4B12185: g_strdup (gstrfuncs.c:363)
==3017==by 0xF79D3: sim_imsi_obtained (sim.c:1535)
==3017==by 0xF7BB3: sim_imsi_cb (sim.c:1594)
==3017==by 0x66C23: at_cimi_cb (sim.c:441)
==3017==by 0xA6B53: at_chat_finish_command (gatchat.c:459)
==3017==by 0xA6D9F: at_chat_handle_command_response (gatchat.c:521)
==3017==by 0xA70AF: have_line (gatchat.c:600)
==3017==by 0xA76DF: new_bytes (gatchat.c:759)
==3017==by 0xABACF: received_data (gatio.c:122)
==3017==by 0xAD093: watch_dispatch (gatmux.c:461)
==3017==by 0xAC5D3: dispatch_sources (gatmux.c:180)
==3017==by 0xAC98F: received_data (gatmux.c:265)
==3017==by 0x4AF606F: g_main_dispatch (gmain.c:3154)
==3017==by 0x4AF606F: g_main_context_dispatch (gmain.c:3769)
==3017==by 0x4AF631D: g_main_context_iterate.isra.4 (gmain.c:3840)
==3017==by 0x4AF658F: g_main_loop_run (gmain.c:4034)
==3017==by 0xBE8AF: main (main.c:261)
---
 src/sim.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 88c0421..155f421 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -3108,8 +3108,6 @@ void ofono_sim_register(struct ofono_sim *sim)
sim->spn_watches = __ofono_watchlist_new(g_free);
sim->simfs = sim_fs_new(sim, sim->driver);
 
-   __ofono_atom_register(sim->atom, sim_unregister);
-
ofono_sim_add_state_watch(sim, sim_ready, sim, NULL);
 
if (sim->state > OFONO_SIM_STATE_NOT_PRESENT)
@@ -3118,6 +3116,8 @@ void ofono_sim_register(struct ofono_sim *sim)
sim->hfp_watch = __ofono_modem_add_atom_watch(modem,
OFONO_ATOM_TYPE_EMULATOR_HFP,
emulator_hfp_watch, sim, NULL);
+
+   __ofono_atom_register(sim->atom, sim_unregister);
 }
 
 void ofono_sim_remove(struct ofono_sim *sim)
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gatchat: Removed unused GAtPPP field

[Slava Monich]

---
 gatchat/gatppp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 5144084..4a80b4b 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -40,7 +40,6 @@
 #include "crc-ccitt.h"
 #include "ppp.h"
 
-#define DEFAULT_MRU1500
 #define DEFAULT_MTU1500
 
 #define PPP_ADDR_FIELD 0xff
@@ -66,7 +65,6 @@ struct _GAtPPP {
struct ppp_chap *chap;
struct ppp_pap *pap;
GAtHDLC *hdlc;
-   gint mru;
gint mtu;
char username[256];
char password[256];
@@ -830,7 +828,6 @@ static GAtPPP *ppp_init_common(gboolean is_server, guint32 
ip)
ppp->fd = -1;
 
/* set options to defaults */
-   ppp->mru = DEFAULT_MRU;
ppp->mtu = DEFAULT_MTU;
 
/* initialize the lcp state */
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gatmux: Remove write watch source at shutdown

[Slava Monich]

Otherwise write_watcher_destroy_notify can be invoked after
GAtMux has been deallocated which results in write after free:

==3952== Invalid write of size 4
==3952==at 0xABF54: write_watcher_destroy_notify (gatmux.c:285)
==3952==by 0x4AF21E7: g_source_callback_unref (gmain.c:1561)
==3952==by 0x4AF2E53: g_source_destroy_internal.constprop.8 (gmain.c:1207)
==3952==by 0x4AF61CF: g_main_dispatch (gmain.c:3177)
==3952==by 0x4AF61CF: g_main_context_dispatch (gmain.c:3769)
==3952==by 0x4AF658F: g_main_loop_run (gmain.c:4034)
==3952==by 0xBDDBB: main (main.c:261)
==3952==  Address 0x50c6cb0 is 8 bytes inside a block of size 4,396 free'd
==3952==at 0x4840B28: free (vg_replace_malloc.c:530)
==3952==by 0xACB53: g_at_mux_unref (gatmux.c:642)
==3952==  Block was alloc'd at
==3952==at 0x4841BF0: calloc (vg_replace_malloc.c:711)
==3952==by 0xAC9DF: g_at_mux_new (gatmux.c:603)
==3952==by 0xADF2F: g_at_mux_new_gsm0710_basic (gatmux.c:1160)
---
 gatchat/gatmux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c
index 909eca6..d492edb 100644
--- a/gatchat/gatmux.c
+++ b/gatchat/gatmux.c
@@ -684,6 +684,9 @@ gboolean g_at_mux_shutdown(GAtMux *mux)
if (mux->read_watch > 0)
g_source_remove(mux->read_watch);
 
+   if (mux->write_watch > 0)
+   g_source_remove(mux->write_watch);
+
for (i = 0; i < MAX_CHANNELS; i++) {
if (mux->dlcs[i] == NULL)
continue;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH v3] atmodem: Query the list of supported s from the modem

[Slava Monich]

Not all modems support all s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
 drivers/atmodem/sim.c | 57 ---
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 6395a04..8665274 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -51,6 +51,7 @@ struct sim_data {
GAtChat *chat;
unsigned int vendor;
guint ready_id;
+   guint passwd_type_mask;
struct at_util_sim_state_query *sim_state_query;
 };
 
@@ -1459,9 +1460,8 @@ static void at_pin_enable(struct ofono_sim *sim,
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
int ret;
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+   if (!(sd->passwd_type_mask & (1 << passwd_type)))
goto error;
 
snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
@@ -1490,10 +1490,8 @@ static void at_change_passwd(struct ofono_sim *sim,
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
int ret;
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len ||
-   at_clck_cpwd_fac[passwd_type] == NULL)
+   if (!(sd->passwd_type_mask & (1 << passwd_type)))
goto error;
 
snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
@@ -1550,9 +1548,8 @@ static void at_query_clck(struct ofono_sim *sim,
struct sim_data *sd = ofono_sim_get_data(sim);
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+   if (!(sd->passwd_type_mask & (1 << passwd_type)))
goto error;
 
snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
@@ -1568,13 +1565,42 @@ error:
CALLBACK_WITH_FAILURE(cb, -1, data);
 }
 
-static gboolean at_sim_register(gpointer user)
+static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
 {
struct ofono_sim *sim = user;
+   struct sim_data *sd = ofono_sim_get_data(sim);
+   GAtResultIter iter;
+   const char *fac;
 
-   ofono_sim_register(sim);
+   if (!ok)
+   goto done;
+
+   g_at_result_iter_init(, result);
+
+   /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
+   if (!g_at_result_iter_next(, "+CLCK:") ||
+   !g_at_result_iter_open_list())
+   goto done;
+
+   /* Clear the default mask */
+   sd->passwd_type_mask = 0;
 
-   return FALSE;
+   /* Set the bits for s that are actually supported */
+   while (g_at_result_iter_next_string(, )) {
+   unsigned int i;
+
+   /* Find it in the list of known s */
+   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
+   if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
+   sd->passwd_type_mask |= (1 << i);
+   DBG("found %s", fac);
+   break;
+   }
+   }
+   }
+
+done:
+   ofono_sim_register(sim);
 }
 
 static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
@@ -1582,6 +1608,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned 
int vendor,
 {
GAtChat *chat = data;
struct sim_data *sd;
+   unsigned int i;
 
sd = g_new0(struct sim_data, 1);
sd->chat = g_at_chat_clone(chat);
@@ -1591,9 +1618,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned 
int vendor,
g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL);
 
ofono_sim_set_data(sim, sd);
-   g_idle_add(at_sim_register, sim);
 
-   return 0;
+   /* s supported by default */
+   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)
+   if (at_clck_cpwd_fac[i])
+   sd->passwd_type_mask |= (1 << i);
+
+   /* Query supported s */
+   return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
+   at_clck_query_cb, sim, NULL) ? 0 : -1;
 }
 
 static void at_sim_remove(struct ofono_sim *sim)
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH v2] atmodem: Query the list of supported s from the modem

[Slava Monich]

On 23/10/17 23:33, Denis Kenzior wrote:

Hi Slava,

On 10/23/2017 03:21 PM, Slava Monich wrote:

Not all modems support all s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
  drivers/atmodem/sim.c | 57 
---

  1 file changed, 45 insertions(+), 12 deletions(-)



drivers/atmodem/sim.c: In function ‘at_clck_query_cb’:
drivers/atmodem/sim.c:1593:17: error: comparison between signed and 
unsigned integer expressions [-Werror=sign-compare]

   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
 ^
drivers/atmodem/sim.c: In function ‘at_sim_probe’:
drivers/atmodem/sim.c:1623:16: error: comparison between signed and 
unsigned integer expressions [-Werror=sign-compare]

  for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)


Apparently we are using slightly different compilers. Will fix.

-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH v2] atmodem: Query the list of supported s from the modem

[Slava Monich]

Not all modems support all s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
 drivers/atmodem/sim.c | 57 ---
 1 file changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 6395a04..06bfb90 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -51,6 +51,7 @@ struct sim_data {
GAtChat *chat;
unsigned int vendor;
guint ready_id;
+   guint passwd_type_mask;
struct at_util_sim_state_query *sim_state_query;
 };
 
@@ -1459,9 +1460,8 @@ static void at_pin_enable(struct ofono_sim *sim,
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
int ret;
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+   if (!(sd->passwd_type_mask & (1 << passwd_type)))
goto error;
 
snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
@@ -1490,10 +1490,8 @@ static void at_change_passwd(struct ofono_sim *sim,
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
int ret;
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len ||
-   at_clck_cpwd_fac[passwd_type] == NULL)
+   if (!(sd->passwd_type_mask & (1 << passwd_type)))
goto error;
 
snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
@@ -1550,9 +1548,8 @@ static void at_query_clck(struct ofono_sim *sim,
struct sim_data *sd = ofono_sim_get_data(sim);
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+   if (!(sd->passwd_type_mask & (1 << passwd_type)))
goto error;
 
snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
@@ -1568,13 +1565,42 @@ error:
CALLBACK_WITH_FAILURE(cb, -1, data);
 }
 
-static gboolean at_sim_register(gpointer user)
+static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
 {
struct ofono_sim *sim = user;
+   struct sim_data *sd = ofono_sim_get_data(sim);
+   GAtResultIter iter;
+   const char *fac;
 
-   ofono_sim_register(sim);
+   if (!ok)
+   goto done;
+
+   g_at_result_iter_init(, result);
+
+   /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
+   if (!g_at_result_iter_next(, "+CLCK:") ||
+   !g_at_result_iter_open_list())
+   goto done;
+
+   /* Clear the default mask */
+   sd->passwd_type_mask = 0;
 
-   return FALSE;
+   /* Set the bits for s that are actually supported */
+   while (g_at_result_iter_next_string(, )) {
+   int i;
+
+   /* Find it in the list of known s */
+   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
+   if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
+   sd->passwd_type_mask |= (1 << i);
+   DBG("found %s", fac);
+   break;
+   }
+   }
+   }
+
+done:
+   ofono_sim_register(sim);
 }
 
 static int at_sim_probe(struct ofono_sim *sim, unsigned int vendor,
@@ -1582,6 +1608,7 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned 
int vendor,
 {
GAtChat *chat = data;
struct sim_data *sd;
+   int i;
 
sd = g_new0(struct sim_data, 1);
sd->chat = g_at_chat_clone(chat);
@@ -1591,9 +1618,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned 
int vendor,
g_at_chat_send(sd->chat, "AT*EPEE=1", NULL, NULL, NULL, NULL);
 
ofono_sim_set_data(sim, sd);
-   g_idle_add(at_sim_register, sim);
 
-   return 0;
+   /* s supported by default */
+   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++)
+   if (at_clck_cpwd_fac[i])
+   sd->passwd_type_mask |= (1 << i);
+
+   /* Query supported s */
+   return g_at_chat_send(sd->chat, "AT+CLCK=?", clck_prefix,
+   at_clck_query_cb, sim, NULL) ? 0 : -1;
 }
 
 static void at_sim_remove(struct ofono_sim *sim)
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] atmodem: Query the list of supported s from the modem

[Slava Monich]

Not all modems support all s (particularly, "PS"), let's be polite
and not ask them for the ones they don't support.
---
 drivers/atmodem/sim.c | 69 ++-
 1 file changed, 57 insertions(+), 12 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 6395a04..effce6e 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -52,6 +52,7 @@ struct sim_data {
unsigned int vendor;
guint ready_id;
struct at_util_sim_state_query *sim_state_query;
+   const char *passwd_fac[OFONO_SIM_PASSWORD_INVALID];
 };
 
 static const char *crsm_prefix[] = { "+CRSM:", NULL };
@@ -1439,7 +1440,7 @@ static void at_lock_unlock_cb(gboolean ok, GAtResult 
*result,
cb(, cbd->data);
 }
 
-static const char *const at_clck_cpwd_fac[] = {
+static const char *const at_clck_cpwd_fac[OFONO_SIM_PASSWORD_INVALID] = {
[OFONO_SIM_PASSWORD_SIM_PIN] = "SC",
[OFONO_SIM_PASSWORD_SIM_PIN2] = "P2",
[OFONO_SIM_PASSWORD_PHSIM_PIN] = "PS",
@@ -1459,13 +1460,13 @@ static void at_pin_enable(struct ofono_sim *sim,
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
int ret;
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+   if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
+   sd->passwd_fac[passwd_type] == NULL)
goto error;
 
snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",%i,\"%s\"",
-   at_clck_cpwd_fac[passwd_type], enable ? 1 : 0, passwd);
+   sd->passwd_fac[passwd_type], enable ? 1 : 0, passwd);
 
ret = g_at_chat_send(sd->chat, buf, none_prefix,
at_lock_unlock_cb, cbd, g_free);
@@ -1490,14 +1491,13 @@ static void at_change_passwd(struct ofono_sim *sim,
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
int ret;
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len ||
-   at_clck_cpwd_fac[passwd_type] == NULL)
+   if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
+   sd->passwd_fac[passwd_type] == NULL)
goto error;
 
snprintf(buf, sizeof(buf), "AT+CPWD=\"%s\",\"%s\",\"%s\"",
-   at_clck_cpwd_fac[passwd_type], old_passwd, new_passwd);
+   sd->passwd_fac[passwd_type], old_passwd, new_passwd);
 
ret = g_at_chat_send(sd->chat, buf, none_prefix,
at_lock_unlock_cb, cbd, g_free);
@@ -1550,13 +1550,13 @@ static void at_query_clck(struct ofono_sim *sim,
struct sim_data *sd = ofono_sim_get_data(sim);
struct cb_data *cbd = cb_data_new(cb, data);
char buf[64];
-   unsigned int len = sizeof(at_clck_cpwd_fac) / sizeof(*at_clck_cpwd_fac);
 
-   if (passwd_type >= len || at_clck_cpwd_fac[passwd_type] == NULL)
+   if (passwd_type >= ARRAY_SIZE(sd->passwd_fac) ||
+   sd->passwd_fac[passwd_type] == NULL)
goto error;
 
snprintf(buf, sizeof(buf), "AT+CLCK=\"%s\",2",
-   at_clck_cpwd_fac[passwd_type]);
+   sd->passwd_fac[passwd_type]);
 
if (g_at_chat_send(sd->chat, buf, clck_prefix,
at_lock_status_cb, cbd, g_free) > 0)
@@ -1568,6 +1568,43 @@ error:
CALLBACK_WITH_FAILURE(cb, -1, data);
 }
 
+static void at_clck_query_cb(gboolean ok, GAtResult *result, gpointer user)
+{
+   struct ofono_sim *sim = user;
+   struct sim_data *sd = ofono_sim_get_data(sim);
+   GAtResultIter iter;
+   const char *fac;
+
+   if (!ok)
+   goto done;
+
+   g_at_result_iter_init(, result);
+
+   /* e.g. +CLCK: ("SC","FD","PN","PU","PP","PC","PF") */
+   if (!g_at_result_iter_next(, "+CLCK:") ||
+   !g_at_result_iter_open_list())
+   goto done;
+
+   memset(sd->passwd_fac, 0, sizeof(sd->passwd_fac));
+
+   while (g_at_result_iter_next_string(, )) {
+   int i;
+
+   /* Find it in the list of known s */
+   for (i = 0; i < ARRAY_SIZE(at_clck_cpwd_fac); i++) {
+   if (!g_strcmp0(at_clck_cpwd_fac[i], fac)) {
+   DBG("found %s", fac);
+   /* at_clck_cpwd_fac[i] is a static string */
+   sd->passwd_fac[i] = at_clck_cpwd_fac[i];
+   break;
+   }
+   }
+   }
+
+done:
+   ofono_sim_register(sim);
+}
+
 static gboolean at_sim_register(gpointer user)
 {
struct ofono_sim *sim = user;
@@ -1591,7 +1628,15 @@ static int at_sim_probe(struct ofono_sim *sim, unsigned 
int vendor,

Re: [PATCHv2] simauth: fixup adding more dbus return checks

[Slava Monich]

Hi James,

On 16/10/17 19:06, James Prestwood wrote:

---
  src/sim-auth.c | 23 +--
  1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/sim-auth.c b/src/sim-auth.c
index b9552b5..9ae5574 100644
--- a/src/sim-auth.c
+++ b/src/sim-auth.c
@@ -427,6 +427,7 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection 
*conn,
DBusMessageIter array;
int i;
struct sim_app_record *app;
+   int rands;
  
  	if (sim->pending)

return __ofono_error_busy(msg);
@@ -436,11 +437,16 @@ static DBusMessage *usim_gsm_authenticate(DBusConnection 
*conn,
if (dbus_message_iter_get_arg_type() != DBUS_TYPE_ARRAY)
return __ofono_error_invalid_format(msg);
  
+	rands = dbus_message_iter_get_element_count();



It appears that dbus_message_iter_get_element_count() requires dbus 
1.9.16 or later, would it be possible to avoid it?


Thanks,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gatmux: Remove finalized watches from the list

[Slava Monich]

Leaving them there may result in invalid reads like this:

==2312== Invalid read of size 4
==2312==at 0xAB8C0: dispatch_sources (gatmux.c:134)
==2312==by 0xAC5D3: channel_close (gatmux.c:479)
==2312==by 0x4AE8885: g_io_channel_shutdown (giochannel.c:523)
==2312==by 0x4AE8A1D: g_io_channel_unref (giochannel.c:240)
==2312==by 0xAC423: watch_finalize (gatmux.c:426)
==2312==by 0x4AF2CC9: g_source_unref_internal (gmain.c:2048)
==2312==by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230)
==2312==by 0x4AF44E1: g_source_destroy (gmain.c:1256)
==2312==by 0x4AF5257: g_source_remove (gmain.c:2282)
==2312==by 0xAB5CB: io_shutdown (gatio.c:325)
==2312==by 0xAB667: g_at_io_unref (gatio.c:345)
==2312==by 0xA72C7: at_chat_unref (gatchat.c:972)
==2312==by 0xA829B: g_at_chat_unref (gatchat.c:1446)
==2312==  Address 0x51420f0 is 56 bytes inside a block of size 60 free'd
==2312==at 0x4840B28: free (vg_replace_malloc.c:530)
==2312==by 0x4AF2D33: g_source_unref_internal (gmain.c:2075)
==2312==by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230)
==2312==by 0x4AF44E1: g_source_destroy (gmain.c:1256)
==2312==by 0x4AF5257: g_source_remove (gmain.c:2282)
==2312==by 0xAB46B: g_at_io_set_write_handler (gatio.c:283)
==2312==by 0xA713F: at_chat_suspend (gatchat.c:938)
==2312==by 0xA72B7: at_chat_unref (gatchat.c:971)
==2312==by 0xA829B: g_at_chat_unref (gatchat.c:1446)
==2312==  Block was alloc'd at
==2312==at 0x4841BF0: calloc (vg_replace_malloc.c:711)
==2312==by 0x4AFB117: g_malloc0 (gmem.c:124)
==2312==by 0x4AF401F: g_source_new (gmain.c:892)
==2312==by 0xAC6A7: channel_create_watch (gatmux.c:506)
==2312==by 0x4AE7C4F: g_io_add_watch_full (giochannel.c:649)
==2312==by 0xAB4EB: g_at_io_set_write_handler (gatio.c:297)
==2312==by 0xA7103: chat_wakeup_writer (gatchat.c:931)
==2312==by 0xA753F: at_chat_send_common (gatchat.c:1045)
==2312==by 0xA850F: g_at_chat_send (gatchat.c:1502)

It's also necessary to add additional references to the sources
for the duration of the dispatch_sources loop because any source
can be removed when any callback is invoked (and not necessarily
the one being dispatched).
---
 gatchat/gatmux.c | 109 +++
 1 file changed, 77 insertions(+), 32 deletions(-)

diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c
index 0e275b5..909eca6 100644
--- a/gatchat/gatmux.c
+++ b/gatchat/gatmux.c
@@ -116,66 +116,109 @@ static inline void debug(GAtMux *mux, const char 
*format, ...)
 
 static void dispatch_sources(GAtMuxChannel *channel, GIOCondition condition)
 {
-   GAtMuxWatch *source;
GSList *c;
GSList *p;
-   GSList *t;
+   GSList *refs;
+
+   /*
+* Don't reference destroyed sources, they may have zero reference
+* count if this function is invoked from the source's finalize
+* callback, in which case incrementing and then decrementing
+* the count would result in double free (first when we decrement
+* the reference count and then when we return from the finalize
+* callback).
+*/
 
p = NULL;
-   c = channel->sources;
+   refs = NULL;
 
-   while (c) {
-   gboolean destroy = FALSE;
+   for (c = channel->sources; c; c = c->next) {
+   GSource *s = c->data;
+
+   if (!g_source_is_destroyed(s)) {
+   GSList *l = g_slist_append(NULL, g_source_ref(s));
+
+   if (p)
+   p->next = l;
+   else
+   refs = l;
 
-   source = c->data;
+   p = l;
+   }
+   }
 
-   debug(channel->mux, "checking source: %p", source);
+   /*
+* Keep the references to all sources for the duration of the loop.
+* Callbacks may add and remove the sources, i.e. channel->sources
+* may keep changing during the loop.
+*/
 
-   if (condition & source->condition) {
+   for (c = refs; c; c = c->next) {
+   GAtMuxWatch *w = c->data;
+   GSource *s = >source;
+
+   if (g_source_is_destroyed(s))
+   continue;
+
+   debug(channel->mux, "checking source: %p", s);
+
+   if (condition & w->condition) {
gpointer user_data = NULL;
GSourceFunc callback = NULL;
-   GSourceCallbackFuncs *cb_funcs;
-   gpointer cb_data;
-   gboolean (*dispatch) (GSource *, GSourceFunc, gpointer);
-
-   debug(channel->mux, "dispatching source: %p", source);
+   GSourceCallbackFuncs *cb_funcs = s->callback_funcs;
+   gpointer cb_data = s->callback_data;
+   gboolean destroy;
 
-

[PATCH] atmodem: Fix use after free in sim_state_cb

[Slava Monich]

==2941== Invalid read of size 4
==2941==at 0x69338: sim_state_cb (sim.c:1301)
==2941==by 0x71DCB: cpin_check_cb (atutil.c:567)
==2941==by 0xA602B: at_chat_finish_command (gatchat.c:459)
==2941==by 0xA6277: at_chat_handle_command_response (gatchat.c:521)
==2941==by 0xA6587: have_line (gatchat.c:600)
==2941==by 0xA6BB7: new_bytes (gatchat.c:759)
==2941==by 0xAAFAF: received_data (gatio.c:124)
==2941==by 0x4AF606F: g_main_dispatch (gmain.c:3154)
==2941==by 0x4AF606F: g_main_context_dispatch (gmain.c:3769)
==2941==by 0x4AF658F: g_main_loop_run (gmain.c:4034)
==2941==by 0xBDDBB: main (main.c:261)
==2941==  Address 0x519c344 is 4 bytes inside a block of size 12 free'd
==2941==at 0x4840B28: free (vg_replace_malloc.c:530)
==2941==by 0x71F33: at_util_sim_state_query_free (atutil.c:613)
==2941==by 0x6930B: sim_state_cb (sim.c:1297)
==2941==by 0x71DCB: cpin_check_cb (atutil.c:567)
==2941==by 0xA602B: at_chat_finish_command (gatchat.c:459)
==2941==by 0xA6277: at_chat_handle_command_response (gatchat.c:521)
==2941==by 0xA6587: have_line (gatchat.c:600)
==2941==by 0xA6BB7: new_bytes (gatchat.c:759)
==2941==by 0xAAFAF: received_data (gatio.c:124)
==2941==by 0x4AF606F: g_main_dispatch (gmain.c:3154)
==2941==by 0x4AF606F: g_main_context_dispatch (gmain.c:3769)
==2941==by 0x4AF658F: g_main_loop_run (gmain.c:4034)
==2941==by 0xBDDBB: main (main.c:261)
---
 drivers/atmodem/sim.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 7c33c22..6395a04 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -1293,14 +1293,15 @@ static void sim_state_cb(gboolean present, gpointer 
user_data)
struct cb_data *cbd = user_data;
struct sim_data *sd = cbd->user;
ofono_sim_lock_unlock_cb_t cb = cbd->cb;
+   void *data = cbd->data;
 
at_util_sim_state_query_free(sd->sim_state_query);
sd->sim_state_query = NULL;
 
if (present == 1)
-   CALLBACK_WITH_SUCCESS(cb, cbd->data);
+   CALLBACK_WITH_SUCCESS(cb, data);
else
-   CALLBACK_WITH_FAILURE(cb, cbd->data);
+   CALLBACK_WITH_FAILURE(cb, data);
 }
 
 static void at_pin_send_cb(gboolean ok, GAtResult *result,
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 2/2] modem: Implement ofono_modem_get_sim

[Slava Monich]

---
 src/modem.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/modem.c b/src/modem.c
index ac361be..4af0551 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -184,6 +184,11 @@ const char *ofono_modem_get_path(struct ofono_modem *modem)
return NULL;
 }
 
+struct ofono_sim *ofono_modem_get_sim(struct ofono_modem *modem)
+{
+   return __ofono_atom_find(OFONO_ATOM_TYPE_SIM, modem);
+}
+
 struct ofono_atom *__ofono_modem_add_atom(struct ofono_modem *modem,
enum ofono_atom_type type,
void (*destruct)(struct ofono_atom *),
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH 1/2] include: Add ofono_modem_get_sim

[Slava Monich]

---
 include/modem.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/modem.h b/include/modem.h
index e40b23e..c93b3d2 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -29,6 +29,7 @@ extern "C" {
 #include 
 
 struct ofono_modem;
+struct ofono_sim;
 
 enum ofono_modem_type {
OFONO_MODEM_TYPE_HARDWARE = 0,
@@ -80,6 +81,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_sim *ofono_modem_get_sim(struct ofono_modem *modem);
 
 void ofono_modem_set_data(struct ofono_modem *modem, void *data);
 void *ofono_modem_get_data(struct ofono_modem *modem);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] netmon: modified api.txt for network monitor agent

[Slava Monich]

Hi Denis,


Hi Slava,


In the Sailfish OS case, one is the positioning helper, and there's 
the factory test app. That makes it at least two. The factory test 
app often plays the role of the second client since it's touching 
pretty much all the important interfaces in the system. And there are 
3rd party apps which we don't control.


See? This wasn't that hard ;)

How does the factory test app use this info?  Does it need actual 
periodic updates or can it simply obtain this via the method call?




Our RIL plugin decides on the update rate internally, based on the 
device state (limiting the rate when the screen turns off).


Different clients have different needs - some need updates only when the 
screen is on (if they are just displaying something on the screen) and 
some may always need updates (a background task collecting some data or 
whatever). The optimal solution may be quite specific to the environment 
where ofono is used.


I'm open to the idea of supporting multiple agents.  We do have to 
figure out who dictates the periodic update interval.  One approach 
might be to have a single master agent and multiple observer agents. 
Feel free to propose something.


You (correctly) said that the agent approach saves some system resources 
by not sending unnecessary signals if no one is interested in this 
information. However, in all other cases the agent (method call) 
approach is less efficient than signals.


ofono calls dbus-daemon, the daemon calls the agent, each call has a 
reply, so the total number of D-Bus packets per notification comes to 
4*n where n is the number of agents (or 2*n if the caller sets 
NO_REPLY_EXPECTED flag).


If you are using signals, the number of packets involved is n+1 (one 
packet from ofono to the daemon and then one to each interested client). 
Obviously, (n+1) > 2*n (let alone 4*n) if and only if n = 0. Not to 
mention that signals are significantly easier to use than (especially, 
multiple) agents.


That said, this NetworkMonitor thing is what it is and you decide how it 
works. If it doesn't suit our needs (and it doesn't) I just implement my 
own (and I did). I'm not in the mood to argue whose implementation is 
better. Everything can be done in so many different ways, so let a 
hundred flowers bloom.


Cheers,
-Slava



Regards,
-Denis



___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] netmon: modified api.txt for network monitor agent

[Slava Monich]

On 07/09/17 19:05, Denis Kenzior wrote:

Hi Slava,

On 09/07/2017 08:06 AM, Slava Monich wrote:

On 07/09/17 12:47, Jonas Bonn wrote:

On 09/07/2017 06:23 AM, Nishanth V wrote:

added new DBUS methods RegisterAgent and UnregisterAgent to
Networkmonitor interface so that any client of ofono can register for
serving cell updates. Added new agent interface NetworkMonitorAgent
with two methods, ServingCellInformationChanged and Release.


My spontaneous reaction to this is that it feels like the wrong 
approach.  Why not make the properties actual DBus properties and 
provide PropertiesChanged events for them? That begets you the same 
functionality for "free".


The implementation also seems to limit the number of clients 
receiving those cell change notifications to just one. Why? In real 
life more than one process may need those.




You keep saying that, but never provide concrete scenarios how this 
could be 'useful'?  I am tempted to simply ignore such feedback in the 
future.




In the Sailfish OS case, one is the positioning helper, and there's the 
factory test app. That makes it at least two. The factory test app often 
plays the role of the second client since it's touching pretty much all 
the important interfaces in the system. And there are 3rd party apps 
which we don't control.


For now I don't see why we would want to have multiple agents 
registered and to keep the implementation simple we will only support 
one.




That's fine, we are using a different interface for getting cell 
information anyway. Go ahead and ignore the feedback.


Cheers,
-Slava


Regards,
-Denis


___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH 1/7] netmon: modified api.txt for network monitor agent

[Slava Monich]

On 07/09/17 12:47, Jonas Bonn wrote:

On 09/07/2017 06:23 AM, Nishanth V wrote:

added new DBUS methods RegisterAgent and UnregisterAgent to
Networkmonitor interface so that any client of ofono can register for
serving cell updates. Added new agent interface NetworkMonitorAgent
with two methods, ServingCellInformationChanged and Release.


My spontaneous reaction to this is that it feels like the wrong 
approach.  Why not make the properties actual DBus properties and 
provide PropertiesChanged events for them?  That begets you the same 
functionality for "free".


The implementation also seems to limit the number of clients receiving 
those cell change notifications to just one. Why? In real life more than 
one process may need those.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] unit: Avoid use of uninitialised data in test-simutil

[Slava Monich]

GTest: run: /testsimutil/ber tlv encode EFpnn
==16777== Conditional jump or move depends on uninitialised value(s)
==16777==at 0x4068CB: ber_tlv_iter_next (simutil.c:369)
==16777==by 0x406C39: ber_tlv_find_by_tag (simutil.c:483)
==16777==by 0x407E1D: sim_eons_add_pnn_record (simutil.c:1027)
==16777==by 0x402C39: test_ber_tlv_builder_efpnn (test-simutil.c:181)
==16777==by 0x4EA3A80: g_test_run_suite_internal
==16777==by 0x4EA3F9A: g_test_run_suite
==16777==by 0x4EA3FD0: g_test_run
==16777==by 0x4042FA: main (test-simutil.c:518)
---
 unit/test-simutil.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/unit/test-simutil.c b/unit/test-simutil.c
index 69dd81e..490e288 100644
--- a/unit/test-simutil.c
+++ b/unit/test-simutil.c
@@ -178,12 +178,12 @@ static void test_ber_tlv_builder_efpnn(void)
ber_tlv_builder_optimize(, NULL, NULL);
 
eons_info = sim_eons_new(1);
-   sim_eons_add_pnn_record(eons_info, 1, efpnn0, sizeof(efpnn0));
+   sim_eons_add_pnn_record(eons_info, 1, efpnn0, 8 + 10);
g_assert(!sim_eons_pnn_is_empty(eons_info));
sim_eons_free(eons_info);
 
eons_info = sim_eons_new(1);
-   sim_eons_add_pnn_record(eons_info, 1, efpnn1, sizeof(efpnn1));
+   sim_eons_add_pnn_record(eons_info, 1, efpnn1, 8 + 6);
g_assert(!sim_eons_pnn_is_empty(eons_info));
sim_eons_free(eons_info);
 }
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sms: Pass const pointer to dispatch_app_datagram

[Slava Monich]

---
 src/sms.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/sms.c b/src/sms.c
index 17c5a9c..b86158e 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -1171,7 +1171,8 @@ static gboolean compute_incoming_msgid(GSList *sms_list,
 static void dispatch_app_datagram(struct ofono_sms *sms,
const struct ofono_uuid *uuid,
int dst, int src,
-   unsigned char *buf, unsigned len,
+   const unsigned char *buf,
+   unsigned int len,
const struct sms_address *addr,
const struct sms_scts *scts)
 {
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] ussd: Switch the state from USER_ACTION to IDLE

[Slava Monich]

... when a USSD notification is received. Some networks
send 0 (no further user action required) after the response
timeout expires. That should result in the user input form
getting removed from the ME screen.
---
 src/ussd.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/src/ussd.c b/src/ussd.c
index 99fa753..84f64c6 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -512,6 +512,20 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int 
status, int dcs,
 
ussd_change_state(ussd, new_state);
goto free;
+   } else if (ussd->state == USSD_STATE_USER_ACTION &&
+   status != OFONO_USSD_STATUS_ACTION_REQUIRED) {
+   ussd_change_state(ussd, USSD_STATE_IDLE);
+
+   if (status == OFONO_USSD_STATUS_NOTIFY && str && str[0]) {
+   const char *path = __ofono_atom_get_path(ussd->atom);
+
+   g_dbus_emit_signal(conn, path,
+   OFONO_SUPPLEMENTARY_SERVICES_INTERFACE,
+   "NotificationReceived", DBUS_TYPE_STRING,
+   , DBUS_TYPE_INVALID);
+   }
+
+   goto free;
} else {
ofono_error("Received an unsolicited USSD but can't handle.");
DBG("USSD is: status: %d, %s", status, str);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] radio-settings: Fix memory leaks in radio_load_settings

[Slava Monich]

Errors returned by g_key_file_get_integer have to be deallocated
by the caller to avoid leaks like these:

==13330== 104 (24 direct, 80 indirect) bytes in 2 blocks are definitely lost
==13330==at 0x483F3EC: malloc (vg_replace_malloc.c)
==13330==by 0x4B020DF: g_malloc (gmem.c)
==13330==by 0x4B17F51: g_slice_alloc (gslice.c)
==13330==by 0x4AE80B9: g_error_new_valist (gerror.c)
==13330==by 0x4AE830B: g_set_error (gerror.c)
==13330==by 0x4AF5681: g_key_file_get_value (gkeyfile.c)
==13330==by 0x4AF6817: g_key_file_get_integer (gkeyfile.c)
==13330==by 0x10CFE3: radio_load_settings (radio-settings.c)
==13330==by 0x10D2E3: ofono_radio_settings_register (radio-settings.c)
---
 src/radio-settings.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/radio-settings.c b/src/radio-settings.c
index b988e3e..4f81a84 100644
--- a/src/radio-settings.c
+++ b/src/radio-settings.c
@@ -856,9 +856,13 @@ static void radio_load_settings(struct 
ofono_radio_settings *rs,
"GsmBand", rs->band_gsm);
}
 
+   if (error) {
+   g_error_free(error);
+   error = NULL;
+   }
+
rs->pending_band_gsm = rs->band_gsm;
 
-   error = NULL;
rs->band_umts = g_key_file_get_integer(rs->settings, SETTINGS_GROUP,
"UmtsBand", );
 
@@ -868,9 +872,13 @@ static void radio_load_settings(struct 
ofono_radio_settings *rs,
"UmtsBand", rs->band_umts);
}
 
+   if (error) {
+   g_error_free(error);
+   error = NULL;
+   }
+
rs->pending_band_umts = rs->band_umts;
 
-   error = NULL;
rs->mode = g_key_file_get_integer(rs->settings, SETTINGS_GROUP,
"TechnologyPreference", );
 
@@ -880,6 +888,11 @@ static void radio_load_settings(struct 
ofono_radio_settings *rs,
"TechnologyPreference", rs->mode);
}
 
+   if (error) {
+   g_error_free(error);
+   error = NULL;
+   }
+
DBG("TechnologyPreference: %d", rs->mode);
DBG("GsmBand: %d", rs->band_gsm);
DBG("UmtsBand: %d", rs->band_umts);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] simfs: Prevent a crash in sim_fs_notify_file_watches

[Slava Monich]

If no file watchers have ever been added, context->file_watches
is NULL and sim_fs_notify_file_watches() should take that into
account.
---
 src/simfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/simfs.c b/src/simfs.c
index 595dbad..37a232a 100644
--- a/src/simfs.c
+++ b/src/simfs.c
@@ -226,6 +226,9 @@ void sim_fs_notify_file_watches(struct sim_fs *fs, int id)
struct ofono_sim_context *context = l->data;
GSList *k;
 
+   if (context->file_watches == NULL)
+   continue;
+
for (k = context->file_watches->items; k; k = k->next) {
struct file_watch *w = k->data;
ofono_sim_file_changed_cb_t notify = w->item.notify;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] test-sms: Fixed memory leaks in the unit test

[Slava Monich]

---
 unit/test-sms.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/unit/test-sms.c b/unit/test-sms.c
index 3d0f016..618d139 100644
--- a/unit/test-sms.c
+++ b/unit/test-sms.c
@@ -1145,6 +1145,8 @@ static void test_assembly(void)
 
reencoded = sms_decode_text(l);
 
+   g_slist_free_full(l, g_free);
+
if (g_test_verbose())
g_printf("ReEncoded:\n%s\n", reencoded);
 
@@ -1304,6 +1306,8 @@ static void test_prepare_concat(gconstpointer data)
g_assert(decoded_str);
g_assert(strcmp(decoded_str, test->str) == 0);
g_free(decoded_str);
+   g_slist_free_full(pdus, g_free);
+   g_slist_free_full(r, g_free);
sms_assembly_free(assembly);
 }
 
@@ -1334,6 +1338,7 @@ static void test_limit(gunichar uni, int target_size, 
gboolean use_16bit)
g_assert(g_utf8_strlen(decoded, -1) == target_size);
 
g_free(decoded);
+   g_slist_free_full(l, g_free);
 
memcpy(utf8 + i, utf8_char, stride);
utf8[i+stride] = '\0';
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] smsutil: Prevent out of bounds reads in cbs_decode_text

[Slava Monich]

Valgrind was complaining about it like this:

==18099== Conditional jump or move depends on uninitialised value(s)
==18099==at 0x4C32281: strspn
==18099==by 0x41286B: cbs_decode_text (smsutil.c:4140)
==18099==by 0x40675C: test_cbs_encode_decode (test-sms.c:1417)
---
 src/smsutil.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index 2212994..24dcfaa 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -4135,12 +4135,13 @@ char *cbs_decode_text(GSList *cbs_list, char 
*iso639_lang)
 */
for (; i < written; i++, bufsize++) {
if (unpacked[i] == '\r') {
-   int t;
+   int j;
 
-   t = strspn((const char *) unpacked + i,
-   "\r");
+   for (j = i + 1; j < written; j++)
+   if (unpacked[j] != '\r')
+   break;
 
-   if (t + i == written)
+   if (j == written)
break;
}
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] smsutil: Fix receiving UTF-16 encoded messages with split 4-byte char

[Slava Monich]

The spec supports UCS2, but in reality UTF-16 is used, which supports
4-byte characters, which could be split into different message
fragments. Accumulate the entire UTF-16 message before converting to
UTF8.

Author: Martin Jones 
---
 src/smsutil.c | 30 ++
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/smsutil.c b/src/smsutil.c
index b6f7348..cfede03 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2218,6 +2218,7 @@ char *sms_decode_text(GSList *sms_list)
const struct sms *sms;
int guess_size = g_slist_length(sms_list);
char *utf8;
+   GByteArray *utf16 = 0;
 
if (guess_size == 1)
guess_size = 160;
@@ -2289,8 +2290,12 @@ char *sms_decode_text(GSList *sms_list)
NULL, NULL, 0,
locking_shift,
single_shift);
+   if (converted) {
+   g_string_append(str, converted);
+   g_free(converted);
+   }
} else {
-   const gchar *from = (const gchar *) (ud + taken);
+   const guint8 *from = ud + taken;
/*
 * According to the spec: A UCS2 character shall not be
 * split in the middle; if the length of the User Data
@@ -2300,15 +2305,32 @@ char *sms_decode_text(GSList *sms_list)
gssize num_ucs2_chars = (udl_in_bytes - taken) >> 1;
num_ucs2_chars = num_ucs2_chars << 1;
 
-   converted = g_convert(from, num_ucs2_chars,
-   "UTF-8//TRANSLIT", "UCS-2BE",
-   NULL, NULL, NULL);
+   /*
+* In theory SMS supports encoding using UCS2 which
+* is 16-bit, however in the real world messages
+* are encoded in UTF-16 which can be 4 bytes and
+* a multiple fragment message can split a 4-byte
+* character in the middle. So accumulate the
+* entire message before converting to UTF-8.
+*/
+   if (!utf16)
+   utf16 = g_byte_array_new();
+
+   g_byte_array_append(utf16, from, num_ucs2_chars);
}
 
+   }
+
+   if (utf16) {
+   char *converted = g_convert_with_fallback((const gchar *)
+   utf16->data, utf16->len,
+   "UTF-8//TRANSLIT", "UTF-16BE",
+   NULL, NULL, NULL, NULL);
if (converted) {
g_string_append(str, converted);
g_free(converted);
}
+   g_byte_array_free(utf16, TRUE);
}
 
utf8 = g_string_free(str, FALSE);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[Slava Monich]

This prevents attached state from getting stuck at 0 like this:

1. Context deactivation is initiated over D-Bus, ctx->pending is set
2. Attached becomes FALSE, context is still marked as active
3. Attached becomes TRUE, gprs_attached_update sets GPRS_FLAG_ATTACHED_UPDATE
4. Deactivation completes, attached is 0, driver_attached is 1

Futher network status updates don't call gprs_attached_update because
driver_attached is still 1, so attached is staying 0 until we lose the
data registration again which may not happen for quite a long time.
---
 src/gprs.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/gprs.c b/src/gprs.c
index 4aa00f9..5f7620b 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -135,6 +135,7 @@ struct pri_context {
struct ofono_gprs *gprs;
 };
 
+static void gprs_attached_update(struct ofono_gprs *gprs);
 static void gprs_netreg_update(struct ofono_gprs *gprs);
 static void gprs_deactivate_next(struct ofono_gprs *gprs);
 
@@ -946,6 +947,16 @@ static void pri_deactivate_callback(const struct 
ofono_error *error, void *data)
ofono_dbus_signal_property_changed(conn, ctx->path,
OFONO_CONNECTION_CONTEXT_INTERFACE,
"Active", DBUS_TYPE_BOOLEAN, );
+
+   /*
+* If "Attached" property was about to be signalled as TRUE but there
+* were still active contexts, try again to signal "Attached" property
+* to registered applications after active contexts have been released.
+*/
+   if (ctx->gprs->flags & GPRS_FLAG_ATTACHED_UPDATE) {
+   ctx->gprs->flags &= ~GPRS_FLAG_ATTACHED_UPDATE;
+   gprs_attached_update(ctx->gprs);
+   }
 }
 
 static void pri_read_settings_callback(const struct ofono_error *error,
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gprs: Check GPRS_FLAG_ATTACHED_UPDATE in pri_deactivate_callback

[Slava Monich]

This prevents attached state from getting stuck at 0 like this:

1. Context deactivation is initiated over D-Bus, ctx->pending is set
2. Attached becomes FALSE, context is still marked as active
3. Attached becomes TRUE, gprs_attached_update sets GPRS_FLAG_ATTACHED_UPDATE
4. Deactivation completes, attached is 0, driver_attached is 1

Futher network status updates don't call gprs_attached_update because
driver_attached is still 1, so attached is staying 0 until we lose the
data registration again which may not happen for quite a long time.
---
 src/gprs.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/gprs.c b/src/gprs.c
index 3a4a819..3d71398 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -135,6 +135,7 @@ struct pri_context {
struct ofono_gprs *gprs;
 };
 
+static void gprs_attached_update(struct ofono_gprs *gprs);
 static void gprs_netreg_update(struct ofono_gprs *gprs);
 static void gprs_deactivate_next(struct ofono_gprs *gprs);
 
@@ -946,6 +947,16 @@ static void pri_deactivate_callback(const struct 
ofono_error *error, void *data)
ofono_dbus_signal_property_changed(conn, ctx->path,
OFONO_CONNECTION_CONTEXT_INTERFACE,
"Active", DBUS_TYPE_BOOLEAN, );
+
+   /*
+* If "Attached" property was about to be signalled as TRUE but there
+* were still active contexts, try again to signal "Attached" property
+* to registered applications after active contexts have been released.
+*/
+   if (ctx->gprs && (ctx->gprs->flags & GPRS_FLAG_ATTACHED_UPDATE)) {
+   ctx->gprs->flags &= ~GPRS_FLAG_ATTACHED_UPDATE;
+   gprs_attached_update(ctx->gprs);
+   }
 }
 
 static void pri_read_settings_callback(const struct ofono_error *error,
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] netmon: Make sure we don't pass NULL message to g_dbus_send_message

[Slava Monich]

Also that we don't lose the reply message.
---
 src/netmon.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/netmon.c b/src/netmon.c
index 9d6de07..eb18b9c 100644
--- a/src/netmon.c
+++ b/src/netmon.c
@@ -199,9 +199,24 @@ static void serving_cell_info_callback(const struct 
ofono_error *error,
struct ofono_netmon *netmon = data;
DBusMessage *reply = netmon->reply;
 
-   if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+   if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+   if (reply)
+   dbus_message_unref(reply);
+
reply = __ofono_error_failed(netmon->pending);
+} else if (!reply) {
+   DBusMessageIter iter;
+   DBusMessageIter dict;
+
+   reply = dbus_message_new_method_return(netmon->pending);
+   dbus_message_iter_init_append(reply, );
+   dbus_message_iter_open_container(, DBUS_TYPE_ARRAY,
+   OFONO_PROPERTIES_ARRAY_SIGNATURE,
+   );
+   dbus_message_iter_close_container(, );
+   }
 
+   netmon->reply = NULL;
__ofono_dbus_pending_reply(>pending, reply);
 }
 
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gprs-context: Remove unused field from struct ofono_gprs_primary_context

[Slava Monich]

---
 include/gprs-context.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/gprs-context.h b/include/gprs-context.h
index c4910f2..ab67326 100644
--- a/include/gprs-context.h
+++ b/include/gprs-context.h
@@ -55,7 +55,6 @@ enum ofono_gprs_auth_method {
 
 struct ofono_gprs_primary_context {
unsigned int cid;
-   int direction;
char apn[OFONO_GPRS_MAX_APN_LENGTH + 1];
char username[OFONO_GPRS_MAX_USERNAME_LENGTH + 1];
char password[OFONO_GPRS_MAX_PASSWORD_LENGTH + 1];
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] main: Make -d option repeatable

[Slava Monich]

Concatenating the patterns makes more sense than using the last
supplied value and leaking the previous allocated patterns.
---
 src/main.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/main.c b/src/main.c
index 46bb90b..b43bb4e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -137,10 +137,19 @@ static gboolean option_version = FALSE;
 static gboolean parse_debug(const char *key, const char *value,
gpointer user_data, GError **error)
 {
-   if (value)
-   option_debug = g_strdup(value);
-   else
+   if (value) {
+   if (option_debug) {
+   char *prev = option_debug;
+
+   option_debug = g_strconcat(prev, ",", value, NULL);
+   g_free(prev);
+   } else {
+   option_debug = g_strdup(value);
+   }
+   } else {
+   g_free(option_debug);
option_debug = g_strdup("*");
+   }
 
return TRUE;
 }
@@ -262,5 +271,7 @@ cleanup:
 
__ofono_log_cleanup();
 
+   g_free(option_debug);
+
return 0;
 }
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] call-forwarding: allow multiple pending GetProperties

[Slava Monich]

The very first call that that every org.ofono.CallForwarding
client makes is GetProperties. With multiple clients, only the
first one was waiting for the completion of the initial query,
all other calls were rejected with org.ofono.Error.InProgress.
In theory, the clients could retry the call later, but in
reality very few clients actually do that.

This patch allows multiple GetProperties requests to be pending
simultaneously.
---
 src/call-forwarding.c | 37 +
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 2746771..762fffe 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -60,6 +60,7 @@ struct ofono_call_forwarding {
GSList *cf_conditions[4];
int flags;
DBusMessage *pending;
+   GSList *pending_get_prop;
int query_next;
int query_end;
struct cf_ss_request *ss_req;
@@ -526,6 +527,14 @@ static DBusMessage *cf_get_properties_reply(DBusMessage 
*msg,
return reply;
 }
 
+static void cf_send_properties(gpointer data, gpointer user_data)
+{
+   DBusMessage *msg = data;
+   DBusMessage *reply = cf_get_properties_reply(msg, user_data);
+
+   __ofono_dbus_pending_reply(, reply);
+}
+
 static void get_query_cf_callback(const struct ofono_error *error, int total,
const struct ofono_call_forwarding_condition *list,
void *data)
@@ -546,8 +555,9 @@ static void get_query_cf_callback(const struct ofono_error 
*error, int total,
}
 
if (cf->query_next == CALL_FORWARDING_TYPE_NOT_REACHABLE) {
-   __ofono_dbus_pending_reply(>pending,
-   cf_get_properties_reply(cf->pending, cf));
+   g_slist_foreach(cf->pending_get_prop, cf_send_properties, cf);
+   g_slist_free(cf->pending_get_prop);
+   cf->pending_get_prop = NULL;
return;
}
 
@@ -574,11 +584,18 @@ static DBusMessage *cf_get_properties(DBusConnection 
*conn, DBusMessage *msg,
if (cf->driver->query == NULL)
return __ofono_error_not_implemented(msg);
 
+   if (cf->pending_get_prop) {
+   /* GetProperties is already in progress */
+   cf->pending_get_prop = g_slist_append(cf->pending_get_prop,
+   dbus_message_ref(msg));
+   return NULL;
+   }
+
if (__ofono_call_forwarding_is_busy(cf) ||
__ofono_ussd_is_busy(cf->ussd))
return __ofono_error_busy(msg);
 
-   cf->pending = dbus_message_ref(msg);
+   cf->pending_get_prop = g_slist_append(NULL, dbus_message_ref(msg));
cf->query_next = 0;
 
get_query_next_cf_cond(cf);
@@ -1261,7 +1278,7 @@ static void cf_unregister_ss_controls(struct 
ofono_call_forwarding *cf)
 
 gboolean __ofono_call_forwarding_is_busy(struct ofono_call_forwarding *cf)
 {
-   return cf->pending ? TRUE : FALSE;
+   return cf->pending || cf->pending_get_prop;
 }
 
 static void sim_cfis_read_cb(int ok, int total_length, int record,
@@ -1371,6 +1388,13 @@ static void sim_cphs_cff_read_cb(int ok, int 
total_length, int record,
DBUS_TYPE_BOOLEAN, _voice);
 }
 
+static void cf_cancel_get_prop(gpointer data)
+{
+   DBusMessage *msg = data;
+
+   __ofono_dbus_pending_reply(, __ofono_error_canceled(msg));
+}
+
 static void call_forwarding_unregister(struct ofono_atom *atom)
 {
struct ofono_call_forwarding *cf = __ofono_atom_get_data(atom);
@@ -1378,6 +1402,11 @@ static void call_forwarding_unregister(struct ofono_atom 
*atom)
DBusConnection *conn = ofono_dbus_get_connection();
struct ofono_modem *modem = __ofono_atom_get_modem(cf->atom);
 
+   if (cf->pending_get_prop) {
+   g_slist_free_full(cf->pending_get_prop, cf_cancel_get_prop);
+   cf->pending_get_prop = NULL;
+   }
+
ofono_modem_remove_interface(modem, OFONO_CALL_FORWARDING_INTERFACE);
g_dbus_unregister_interface(conn, path,
OFONO_CALL_FORWARDING_INTERFACE);
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Query the status of SC facility lock

[Slava Monich]

Resending after adjusting code style.
---
 src/sim.c | 72 +++
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index aedc617..e2da78d 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2449,58 +2449,50 @@ static void sim_free_state(struct ofono_sim *sim)
sim_free_main_state(sim);
 }
 
-static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
-   ofono_bool_t status,
-   void *data)
+static void sim_set_locked_pin(struct ofono_sim *sim,
+   enum ofono_sim_password_type type, gboolean locked)
 {
-   struct ofono_sim *sim = data;
-   DBusConnection *conn = ofono_dbus_get_connection();
-   const char *path = __ofono_atom_get_path(sim->atom);
char **locked_pins;
 
-   if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-   ofono_error("Querying Facility Lock for IMSI Lock failed");
+   if (sim->locked_pins[type] == locked)
return;
-   }
-
-   sim->locked_pins[OFONO_SIM_PASSWORD_PHSIM_PIN] = status;
 
+   sim->locked_pins[type] = locked;
locked_pins = get_locked_pins(sim);
 
-   ofono_dbus_signal_array_property_changed(conn,
-   path,
-   OFONO_SIM_MANAGER_INTERFACE,
-   "LockedPins", DBUS_TYPE_STRING,
-   _pins);
+   ofono_dbus_signal_array_property_changed(ofono_dbus_get_connection(),
+   __ofono_atom_get_path(sim->atom),
+   OFONO_SIM_MANAGER_INTERFACE, "LockedPins",
+   DBUS_TYPE_STRING, _pins);
 
g_strfreev(locked_pins);
 }
 
-static void sim_query_fac_networklock_cb(const struct ofono_error *error,
-   ofono_bool_t status,
-   void *data)
+static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
+   ofono_bool_t status, void *data)
 {
-   struct ofono_sim *sim = data;
-   DBusConnection *conn = ofono_dbus_get_connection();
-   const char *path = __ofono_atom_get_path(sim->atom);
-   char **locked_pins;
-
-   if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-   ofono_error("Querying Facility Lock for Network Lock failed");
+   if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
return;
-   }
 
-   sim->locked_pins[OFONO_SIM_PASSWORD_PHNET_PIN] = status;
+   sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHSIM_PIN, status);
+}
 
-   locked_pins = get_locked_pins(sim);
+static void sim_query_fac_networklock_cb(const struct ofono_error *error,
+   ofono_bool_t status, void *data)
+{
+   if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+   return;
 
-   ofono_dbus_signal_array_property_changed(conn,
-   path,
-   OFONO_SIM_MANAGER_INTERFACE,
-   "LockedPins", DBUS_TYPE_STRING,
-   _pins);
+   sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHNET_PIN, status);
+}
 
-   g_strfreev(locked_pins);
+static void sim_query_fac_pinlock_cb(const struct ofono_error *error,
+   ofono_bool_t status, void *data)
+{
+   if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+   return;
+
+   sim_set_locked_pin(data, OFONO_SIM_PASSWORD_SIM_PIN, status);
 }
 
 void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
@@ -2529,14 +2521,20 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, 
ofono_bool_t inserted)
call_state_watches(sim);
 
if (inserted) {
-   sim->driver->query_facility_lock(sim,
+   if (sim->driver->query_facility_lock) {
+   sim->driver->query_facility_lock(sim,
OFONO_SIM_PASSWORD_PHSIM_PIN,
sim_query_fac_imsilock_cb, sim);
 
-   sim->driver->query_facility_lock(sim,
+   sim->driver->query_facility_lock(sim,
OFONO_SIM_PASSWORD_PHNET_PIN,
sim_query_fac_networklock_cb, sim);
 
+   sim->driver->query_facility_lock(sim,
+   OFONO_SIM_PASSWORD_SIM_PIN,
+   sim_query_fac_pinlock_cb, sim);
+   }
+
sim_initialize(sim);
} else {
sim->pin_type = OFONO_SIM_PASSWORD_NONE;
-- 
1.9.1

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Query the status of SC facility lock

[Slava Monich]

Otherwise "pin" entry in LockedPins may be lost after ofono is restarted.
---
 src/sim.c | 80 +--
 1 file changed, 37 insertions(+), 43 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index aedc617..634acfc 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2449,58 +2449,47 @@ static void sim_free_state(struct ofono_sim *sim)
sim_free_main_state(sim);
 }
 
-static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
-   ofono_bool_t status,
-   void *data)
+static void sim_set_locked_pin(struct ofono_sim *sim,
+   enum ofono_sim_password_type type, gboolean locked)
 {
-   struct ofono_sim *sim = data;
-   DBusConnection *conn = ofono_dbus_get_connection();
-   const char *path = __ofono_atom_get_path(sim->atom);
-   char **locked_pins;
+   if (sim->locked_pins[type] != locked) {
+   char **locked_pins;
 
-   if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-   ofono_error("Querying Facility Lock for IMSI Lock failed");
-   return;
-   }
+   sim->locked_pins[type] = locked;
+   locked_pins = get_locked_pins(sim);
 
-   sim->locked_pins[OFONO_SIM_PASSWORD_PHSIM_PIN] = status;
+   ofono_dbus_signal_array_property_changed(
+   ofono_dbus_get_connection(),
+   __ofono_atom_get_path(sim->atom),
+   OFONO_SIM_MANAGER_INTERFACE, "LockedPins",
+   DBUS_TYPE_STRING, _pins);
 
-   locked_pins = get_locked_pins(sim);
-
-   ofono_dbus_signal_array_property_changed(conn,
-   path,
-   OFONO_SIM_MANAGER_INTERFACE,
-   "LockedPins", DBUS_TYPE_STRING,
-   _pins);
+   g_strfreev(locked_pins);
+   }
+}
 
-   g_strfreev(locked_pins);
+static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
+   ofono_bool_t status, void *data)
+{
+   if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+   sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHSIM_PIN, status);
+   }
 }
 
 static void sim_query_fac_networklock_cb(const struct ofono_error *error,
-   ofono_bool_t status,
-   void *data)
+   ofono_bool_t status, void *data)
 {
-   struct ofono_sim *sim = data;
-   DBusConnection *conn = ofono_dbus_get_connection();
-   const char *path = __ofono_atom_get_path(sim->atom);
-   char **locked_pins;
-
-   if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-   ofono_error("Querying Facility Lock for Network Lock failed");
-   return;
+   if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+   sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHNET_PIN, status);
}
+}
 
-   sim->locked_pins[OFONO_SIM_PASSWORD_PHNET_PIN] = status;
-
-   locked_pins = get_locked_pins(sim);
-
-   ofono_dbus_signal_array_property_changed(conn,
-   path,
-   OFONO_SIM_MANAGER_INTERFACE,
-   "LockedPins", DBUS_TYPE_STRING,
-   _pins);
-
-   g_strfreev(locked_pins);
+static void sim_query_fac_pinlock_cb(const struct ofono_error *error,
+   ofono_bool_t status, void *data)
+{
+   if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+   sim_set_locked_pin(data, OFONO_SIM_PASSWORD_SIM_PIN, status);
+   }
 }
 
 void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
@@ -2529,14 +2518,19 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, 
ofono_bool_t inserted)
call_state_watches(sim);
 
if (inserted) {
-   sim->driver->query_facility_lock(sim,
+   if (sim->driver->query_facility_lock) {
+   sim->driver->query_facility_lock(sim,
OFONO_SIM_PASSWORD_PHSIM_PIN,
sim_query_fac_imsilock_cb, sim);
 
-   sim->driver->query_facility_lock(sim,
+   sim->driver->query_facility_lock(sim,
OFONO_SIM_PASSWORD_PHNET_PIN,
sim_query_fac_networklock_cb, sim);
 
+   sim->driver->query_facility_lock(sim,
+   OFONO_SIM_PASSWORD_SIM_PIN,
+   sim_query_fac_pinlock_cb, sim);
+   }
sim_initialize(sim);
} else {
sim->pin_type = OFONO_SIM_PASSWORD_NONE;
-- 
1.9.1

___
ofono mailing list

Re: [PATCH] gprs: Setup route for mmsc if there's no mms proxy

[Slava Monich]

And don't setup mms route at all if mmsc/proxy host name is not in
dotted IPv4 format.
---
  src/gprs.c | 25 -
  1 file changed, 20 insertions(+), 5 deletions(-)


Sorry, somehow I missed the reply to this patch and just accidentally 
found it (the reply) at mail-archive.com.


This patch has to do the the case when no MessageProxy is provided for 
the MMS context. In that case ofono would set up host route for 
255.255.255.255 (INADDR_NONE returned by inet_addr for the empty string) 
for the MMS interface when it comes up.


With these changes the whole thing has a chance to work if MessageCenter 
URL contains IP address in the host part. In any case, configuring host 
route for 255.255.255.255 doesn't make any sense.


Regards,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] gprs: Setup route for mmsc if there's no mms proxy

[Slava Monich]

And don't setup mms route at all if mmsc/proxy host name is not in
dotted IPv4 format.
---
 src/gprs.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index 05ab499..9c643b9 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -585,13 +585,16 @@ static void pri_context_signal_settings(struct 
pri_context *ctx,
context_settings_append_ipv6);
 }
 
-static void pri_parse_proxy(struct pri_context *ctx, const char *proxy)
+static gboolean pri_parse_proxy(struct pri_context *ctx, const char *proxy)
 {
char *scheme, *host, *port, *path;
 
+   if (proxy[0] == 0)
+   return FALSE;
+
scheme = g_strdup(proxy);
if (scheme == NULL)
-   return;
+   return FALSE;
 
host = strstr(scheme, ://);
if (host != NULL) {
@@ -604,7 +607,7 @@ static void pri_parse_proxy(struct pri_context *ctx, const 
char *proxy)
ctx-proxy_port = 80;
else {
g_free(scheme);
-   return;
+   return FALSE;
}
} else {
host = scheme;
@@ -626,10 +629,16 @@ static void pri_parse_proxy(struct pri_context *ctx, 
const char *proxy)
}
}
 
+   if (host[0] == 0) {
+   g_free(scheme);
+   return FALSE;
+   }
+
g_free(ctx-proxy_host);
ctx-proxy_host = g_strdup(host);
 
g_free(scheme);
+   return TRUE;
 }
 
 static void pri_ifupdown(const char *interface, ofono_bool_t active)
@@ -715,11 +724,16 @@ static void pri_setproxy(const char *interface, const 
char *proxy)
 {
struct rtentry rt;
struct sockaddr_in addr;
+   in_addr_t proxy_addr;
int sk;
 
if (interface == NULL)
return;
 
+   proxy_addr = inet_addr(proxy);
+   if (proxy_addr == INADDR_NONE)
+   return;
+
sk = socket(PF_INET, SOCK_DGRAM, 0);
if (sk  0)
return;
@@ -730,7 +744,7 @@ static void pri_setproxy(const char *interface, const char 
*proxy)
 
memset(addr, 0, sizeof(addr));
addr.sin_family = AF_INET;
-   addr.sin_addr.s_addr = inet_addr(proxy);
+   addr.sin_addr.s_addr = proxy_addr;
memcpy(rt.rt_dst, addr, sizeof(rt.rt_dst));
 
memset(addr, 0, sizeof(addr));
@@ -792,7 +806,8 @@ static void pri_update_mms_context_settings(struct 
pri_context *ctx)
if (ctx-message_proxy)
settings-ipv4-proxy = g_strdup(ctx-message_proxy);
 
-   pri_parse_proxy(ctx, ctx-message_proxy);
+   if (!pri_parse_proxy(ctx, ctx-message_proxy))
+   pri_parse_proxy(ctx, ctx-message_center);
 
DBG(proxy %s port %u, ctx-proxy_host, ctx-proxy_port);
 
-- 
1.8.3.2

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] network: Add NetworkRegistration.OperatorsChanged signal

[Slava Monich]

This signal gets emitted when operator list has changed.
It contains the current list of operators.
---
 doc/network-api.txt |  5 +
 src/network.c   | 44 +++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/doc/network-api.txt b/doc/network-api.txt
index 83a2bc0..d635ba7 100644
--- a/doc/network-api.txt
+++ b/doc/network-api.txt
@@ -57,6 +57,11 @@ Signals  PropertyChanged(string property, 
variant value)
This signal indicates a changed value of the given
property.
 
+   OperatorsChanged(array{object,dict})
+
+   Signal that gets emitted when operator list has
+   changed. It contains the current list of operators.
+
 Properties string Mode [readonly]
 
The current registration mode. The default of this
diff --git a/src/network.c b/src/network.c
index d1bfca6..1b8b759 100644
--- a/src/network.c
+++ b/src/network.c
@@ -934,6 +934,40 @@ static void append_operator_struct_list(struct 
ofono_netreg *netreg,
dbus_free_string_array(children);
 }
 
+static void network_signal_operators_changed(struct ofono_netreg *netreg)
+{
+   const char *path = __ofono_atom_get_path(netreg-atom);
+   DBusConnection *conn = ofono_dbus_get_connection();
+   DBusMessage *signal;
+   DBusMessageIter iter;
+   DBusMessageIter array;
+
+   signal = dbus_message_new_signal(path,
+   OFONO_NETWORK_REGISTRATION_INTERFACE, OperatorsChanged);
+   if (signal == NULL) {
+   ofono_error(Unable to allocate new 
+   OFONO_NETWORK_REGISTRATION_INTERFACE
+   .OperatorsChanged signal);
+   return;
+   }
+
+   dbus_message_iter_init_append(signal, iter);
+   dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
+   DBUS_STRUCT_BEGIN_CHAR_AS_STRING
+   DBUS_TYPE_OBJECT_PATH_AS_STRING
+   DBUS_TYPE_ARRAY_AS_STRING
+   DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
+   DBUS_TYPE_STRING_AS_STRING
+   DBUS_TYPE_VARIANT_AS_STRING
+   DBUS_DICT_ENTRY_END_CHAR_AS_STRING
+   DBUS_STRUCT_END_CHAR_AS_STRING,
+   array);
+   append_operator_struct_list(netreg, array);
+   dbus_message_iter_close_container(iter, array);
+
+   g_dbus_send_message(conn, signal);
+}
+
 static void operator_list_callback(const struct ofono_error *error, int total,
const struct ofono_network_operator *list,
void *data)
@@ -942,6 +976,7 @@ static void operator_list_callback(const struct ofono_error 
*error, int total,
DBusMessage *reply;
DBusMessageIter iter;
DBusMessageIter array;
+   gboolean changed;
 
if (error-type != OFONO_ERROR_TYPE_NO_ERROR) {
DBG(Error occurred during operator list);
@@ -950,7 +985,7 @@ static void operator_list_callback(const struct ofono_error 
*error, int total,
return;
}
 
-   update_operator_list(netreg, total, list);
+   changed = update_operator_list(netreg, total, list);
 
reply = dbus_message_new_method_return(netreg-pending);
 
@@ -970,6 +1005,11 @@ static void operator_list_callback(const struct 
ofono_error *error, int total,
dbus_message_iter_close_container(iter, array);
 
__ofono_dbus_pending_reply(netreg-pending, reply);
+
+   DBG(operator list %schanged, changed ?  : not );
+
+   if (changed)
+   network_signal_operators_changed(netreg);
 }
 
 static DBusMessage *network_scan(DBusConnection *conn,
@@ -1041,6 +1081,8 @@ static const GDBusMethodTable 
network_registration_methods[] = {
 static const GDBusSignalTable network_registration_signals[] = {
{ GDBUS_SIGNAL(PropertyChanged,
GDBUS_ARGS({ name, s }, { value, v })) },
+   { GDBUS_SIGNAL(OperatorsChanged,
+   GDBUS_ARGS({ operators, a(oa{sv})})) },
{ }
 };
 
-- 
1.8.3.2

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] sim: Handle multiple invocations of sim_imsi_obtained

[Slava Monich]

To avoid leaking memory and issuing unnecessary D-Bus signals
---
 src/sim.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index edae5eb..1b0cb80 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -1425,11 +1425,18 @@ static void sim_set_ready(struct ofono_sim *sim)
 
 static void sim_imsi_obtained(struct ofono_sim *sim, const char *imsi)
 {
-   DBusConnection *conn = ofono_dbus_get_connection();
-   const char *path = __ofono_atom_get_path(sim-atom);
+   DBusConnection *conn;
+   const char *path;
+
+   if (sim-imsi  !strcmp(sim-imsi, imsi))
+   return;
 
+   DBG(%s, imsi);
+   g_free(sim-imsi);
sim-imsi = g_strdup(imsi);
 
+   conn = ofono_dbus_get_connection();
+   path = __ofono_atom_get_path(sim-atom);
ofono_dbus_signal_property_changed(conn, path,
OFONO_SIM_MANAGER_INTERFACE,
SubscriberIdentity,
-- 
1.8.3.2

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] sim: Handle multiple invocations of sim_imsi_obtained

[Slava Monich]

Hi Denis,


To avoid leaking memory and issuing unnecessary D-Bus signals
---
  src/sim.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index edae5eb..1b0cb80 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -1425,11 +1425,18 @@ static void sim_set_ready(struct ofono_sim *sim)
  
  static void sim_imsi_obtained(struct ofono_sim *sim, const char *imsi)

  {
-   DBusConnection *conn = ofono_dbus_get_connection();
-   const char *path = __ofono_atom_get_path(sim-atom);
+   DBusConnection *conn;
+   const char *path;
+
+   if (sim-imsi  !strcmp(sim-imsi, imsi))
+   return;

Calling this function a second time is an error.  So this condition
looks highly suspect.  What issue are you trying to address?


sim_imsi_obtained() is invoked from two places: sim_imsi_cb() and 
sim_efimsi_cb(). sim_imsi_cb() in turn is a completion for 
sim_retrieve_imsi() which is invoked from 5 other asynchronous 
callbacks, mostly triggered by a number of ofono_sim_read() calls. I 
don't claim that I fully understand what's going on and I haven't 
analyzed all the call sequences but it surely is invoked twice in our 
setup. Any suggestions on where the problem might be?


Thanks,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] mbpi: Parse gsm provider name

[Slava Monich]

Hi Denis,


Hi Slava,


Operators share mcc/mnc meaning that automatically finding the best AP
(the one most likely to work) from the database may involve some sort of
fuzzy logic (partial or case-insensitive name comparison or something
like this) implemented in a product-specific provisioning plugin. Making
things work out-the-box translates into fewer customer complaints,
better user experience and is generally the right thing to do.

I was trying to share a piece of code that doesn't involve any
product-specific logic and yet might be useful to others.

Please let me know if I should give it another try, otherwise I won't
waste anyone's time and this code will just remain in our branch of ofono.

So you're trying to parse and return provider name information from mbpi
and return it back to the caller.  Then the caller will try to
'fuzzy-logic' and match it against the SPN.  Right?  Why not make things
easy on yourself and fix the database for the operators you need?  But I
digress.

Feel free to try.  If you can do this cleanly, then I have no problem
with it in principle.



Even if the operator database was 100% accurate (which is practically 
impossible) something would still have to be done about the operators 
sharing MCC/MNC and yet using different AP settings.


I'll give it another try.

Thanks,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

[PATCH] mbpi: Return provider name to the provisioning plugin

[Slava Monich]

Second try, this time not touching core interfaces.
---
 plugins/mbpi.c  | 59 -
 plugins/mbpi.h  | 14 -
 plugins/provision.c | 15 +++---
 tools/lookup-apn.c  |  5 +++--
 4 files changed, 77 insertions(+), 16 deletions(-)

diff --git a/plugins/mbpi.c b/plugins/mbpi.c
index dff8752..0b30a54 100644
--- a/plugins/mbpi.c
+++ b/plugins/mbpi.c
@@ -53,6 +53,7 @@ enum MBPI_ERROR {
 struct gsm_data {
const char *match_mcc;
const char *match_mnc;
+   char *provider_name;
GSList *apns;
gboolean match_found;
gboolean allow_duplicates;
@@ -82,7 +83,7 @@ static GQuark mbpi_error_quark(void)
return g_quark_from_static_string(ofono-mbpi-error-quark);
 }
 
-void mbpi_ap_free(struct ofono_gprs_provision_data *ap)
+void mbpi_ap_free(struct mbpi_provision_data *ap)
 {
g_free(ap-name);
g_free(ap-apn);
@@ -90,6 +91,7 @@ void mbpi_ap_free(struct ofono_gprs_provision_data *ap)
g_free(ap-password);
g_free(ap-message_proxy);
g_free(ap-message_center);
+   g_free(ap-provider_name);
 
g_free(ap);
 }
@@ -117,6 +119,7 @@ static void text_handler(GMarkupParseContext *context,
 {
char **string = userdata;
 
+   g_free(*string);
*string = g_strndup(text, text_len);
 }
 
@@ -164,7 +167,7 @@ static void apn_start(GMarkupParseContext *context, const 
gchar *element_name,
const gchar **attribute_values,
gpointer userdata, GError **error)
 {
-   struct ofono_gprs_provision_data *apn = userdata;
+   struct mbpi_provision_data *apn = userdata;
 
if (g_str_equal(element_name, name))
g_markup_parse_context_push(context, text_parser, apn-name);
@@ -263,7 +266,7 @@ static void apn_handler(GMarkupParseContext *context, 
struct gsm_data *gsm,
const gchar **attribute_values,
GError **error)
 {
-   struct ofono_gprs_provision_data *ap;
+   struct mbpi_provision_data *ap;
const char *apn;
int i;
 
@@ -287,7 +290,8 @@ static void apn_handler(GMarkupParseContext *context, 
struct gsm_data *gsm,
return;
}
 
-   ap = g_new0(struct ofono_gprs_provision_data, 1);
+   ap = g_new0(struct mbpi_provision_data, 1);
+   ap-provider_name = g_strdup(gsm-provider_name);
ap-apn = g_strdup(apn);
ap-type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
ap-proto = OFONO_GPRS_PROTO_IP;
@@ -349,7 +353,7 @@ static void gsm_end(GMarkupParseContext *context, const 
gchar *element_name,
gpointer userdata, GError **error)
 {
struct gsm_data *gsm;
-   struct ofono_gprs_provision_data *ap;
+   struct mbpi_provision_data *ap;
 
if (!g_str_equal(element_name, apn))
return;
@@ -364,7 +368,7 @@ static void gsm_end(GMarkupParseContext *context, const 
gchar *element_name,
GSList *l;
 
for (l = gsm-apns; l; l = l-next) {
-   struct ofono_gprs_provision_data *pd = l-data;
+   struct mbpi_provision_data *pd = l-data;
 
if (pd-type != ap-type)
continue;
@@ -454,7 +458,7 @@ static const GMarkupParser provider_parser = {
NULL,
 };
 
-static void toplevel_gsm_start(GMarkupParseContext *context,
+static void gsm_provider_start(GMarkupParseContext *context,
const gchar *element_name,
const gchar **atribute_names,
const gchar **attribute_values,
@@ -462,19 +466,53 @@ static void toplevel_gsm_start(GMarkupParseContext 
*context,
 {
struct gsm_data *gsm = userdata;
 
-   if (g_str_equal(element_name, gsm)) {
+   if (g_str_equal(element_name, name)) {
+   g_free(gsm-provider_name);
+   gsm-provider_name = NULL;
+   g_markup_parse_context_push(context, text_parser,
+   gsm-provider_name);
+   } else if (g_str_equal(element_name, gsm)) {
gsm-match_found = FALSE;
g_markup_parse_context_push(context, gsm_parser, gsm);
} else if (g_str_equal(element_name, cdma))
g_markup_parse_context_push(context, skip_parser, NULL);
 }
 
+static void gsm_provider_end(GMarkupParseContext *context,
+   const gchar *element_name,
+   gpointer userdata, GError **error)
+{
+   if (g_str_equal(element_name, name) ||
+   g_str_equal(element_name, gsm) ||
+   g_str_equal(element_name, cdma))
+   g_markup_parse_context_pop(context);
+}
+
+static const GMarkupParser gsm_provider_parser = {
+   gsm_provider_start,
+   

Re: [PATCH] mbpi: Parse gsm provider name

[Slava Monich]

Hi Denis,
It's for custom provisioning plugins.

Service provider name is passed to the provisioning plugin but not used 
there at all. This patch makes it possible to utilize it for AP matching.


Regards,
-Slava


On 17/04/14 07:07, Denis Kenzior wrote:

Hi Slava,

On 04/16/2014 09:00 AM, Slava Monich wrote:

This gives the provisioning plugin more information to work with and
improves the chance of automatically picking the right AP in case if
we have more than one for the same mcc/mnc combination.
---
  include/gprs-provision.h |  1 +
  plugins/mbpi.c   | 47 +++
  2 files changed, 44 insertions(+), 4 deletions(-)


I fail to see the point for this patch.  It makes no sense on its own.


diff --git a/include/gprs-provision.h b/include/gprs-provision.h
index e9eec61..0129cd0 100644
--- a/include/gprs-provision.h
+++ b/include/gprs-provision.h
@@ -31,6 +31,7 @@ extern C {
  struct ofono_gprs_provision_data {
enum ofono_gprs_context_type type;
enum ofono_gprs_proto proto;
+   char *provider_name;

I fail to see why this would be required here?  It is not utilized
anywhere else...

Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono



___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] mbpi: Parse gsm provider name

[Slava Monich]

Hi Denis,


Hi Slava,

Please do not top-post on this mailing list.


ah, sorry.



On 04/17/2014 03:24 AM, Slava Monich wrote:

Hi Denis,
It's for custom provisioning plugins.

Service provider name is passed to the provisioning plugin but not used
there at all. This patch makes it possible to utilize it for AP matching.


I still do not understand.  SPN is passed to the provisioning plugin via
get_settings.  I do not see any need for it to be stored in
ofono_gprs_provision_data struct.  How is it going to be used by the core?


I agree that there is no need to for it to be returned by the 
provisioning plugin to the core but it's the same structure used by 
mbpi.c to return the result of parsing serviceproviders.xml to the 
provisioning plugin. Side effect of sharing the structure between two 
interfaces.


If instead I changed the interface between mbpi and provisioning plugin 
and kept the interface with the core intact, would that look acceptable 
to you?


Thanks,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Re: [PATCH] mbpi: Parse gsm provider name

[Slava Monich]

Hi Denis,

If instead I changed the interface between mbpi and provisioning plugin
and kept the interface with the core intact, would that look acceptable
to you?

Perhaps, but I'm still failing to see why the service provider name as
entered in MBPI is useful to anyone.


Operators share mcc/mnc meaning that automatically finding the best AP 
(the one most likely to work) from the database may involve some sort of 
fuzzy logic (partial or case-insensitive name comparison or something 
like this) implemented in a product-specific provisioning plugin. Making 
things work out-the-box translates into fewer customer complaints, 
better user experience and is generally the right thing to do.


I was trying to share a piece of code that doesn't involve any 
product-specific logic and yet might be useful to others.


Please let me know if I should give it another try, otherwise I won't 
waste anyone's time and this code will just remain in our branch of ofono.


Thanks,
-Slava

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono