Merge lp:~phablet-team/ofono/ofono-mwi-and-bugs into lp:~phablet-team/ofono/ubuntu

Proposed by Tony Espy
Status: Merged
Merged at revision: 6854
Proposed branch: lp:~phablet-team/ofono/ofono-mwi-and-bugs
Merge into: lp:~phablet-team/ofono/ubuntu
Diff against target: 280 lines (+150/-19)
7 files modified
debian/changelog (+15/-1)
drivers/rilmodem/sim.c (+6/-1)
drivers/rilmodem/voicecall.c (+22/-7)
gril/grilreply.c (+12/-9)
plugins/ril.c (+5/-0)
src/smsutil.c (+1/-1)
unit/test-grilreply.c (+89/-0)
To merge this branch: bzr merge lp:~phablet-team/ofono/ofono-mwi-and-bugs
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Needs Fixing
Ricardo Salveti Pending
Review via email: mp+204587@code.launchpad.net

This proposal supersedes a proposal from 2014-01-30.

Commit message

* gril, rilmodem/sim, unit: fix SIM IO crash (LP: #1268743)
   - cleanup gril_reply_parse_sim_io() to add malformed parcel
     check and fix memory leak.
   - add check for null hex_response in ril_file_io_cb(), as
     the emulator can return such responses.
   - add additional unit tests to cover crash scenarios.
 * ril, src: enable message-waiting-interface
   - register message_waiting atom in ril plugin
   - fix sms_mwi_dcs_decode bug which prevented incoming message
     waiting indications from being set.
 * rilmodem/voicecall: fix call-decline bug (LP: #1260988)
   Send a RIL_REQUEST_HANGUP_WAITING_OR_BACKGROUND instead of
   RIL_REQUEST_HANGUP for incoming calls.

Description of the change

This change includes a couple of upstream changes:

1. Fixes a SIM IO crash that only occurs on the emulator (LP: #1268743).

2. Enables the message-waiting interface in the ril device plugin.
   This exposes a new ofono DBus interface ( see doc/message-waiting-api.txt
   for details ) which allows the upper layers to detect when new voicemails
   have landed in the user's voicemailbox. It also exposes the voicemailbox
   phone number so that the dialer app can implenent a standard voicemail
   shortcut ( long press on "1" ).

3. Fixe a bug with certain operators where declining an incoming call leads to
   a fast busy signal instead of voicemail ( LP: #1260988 ).

Tested on mako, maguro, and the emulator using build #121. Please see bug descriptions
for testing instruction, and the specified documentation for the new message-waiting
API.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2014-01-21 19:01:39 +0000
+++ debian/changelog 2014-02-03 22:25:52 +0000
@@ -1,4 +1,5 @@
1ofono (1.12+bzr6851-0ubuntu2) UNRELEASED; urgency=low1ofono (1.12+bzr6851-0ubuntu2) UNRELEASED; urgency=low
2
2 [ Tony Espy ]3 [ Tony Espy ]
3 * unit: fix fail-to-build on powerpc4 * unit: fix fail-to-build on powerpc
4 The previously released version enabled -Wall,5 The previously released version enabled -Wall,
@@ -8,8 +9,21 @@
8 systems via an ifdef, which leaves unused functions9 systems via an ifdef, which leaves unused functions
9 and data. This change exends the ifdef to cover10 and data. This change exends the ifdef to cover
10 the unused functions and test data.11 the unused functions and test data.
12 * gril, rilmodem/sim, unit: fix SIM IO crash (LP: #1268743)
13 - cleanup gril_reply_parse_sim_io() to add malformed parcel
14 check and fix memory leak.
15 - add check for null hex_response in ril_file_io_cb(), as
16 the emulator can return such responses.
17 - add additional unit tests to cover crash scenarios.
18 * ril, src: enable message-waiting-interface
19 - register message_waiting atom in ril plugin
20 - fix sms_mwi_dcs_decode bug which prevented incoming message
21 waiting indications from being set.
22 * rilmodem/voicecall: fix call-decline bug (LP: #1260988)
23 Send a RIL_REQUEST_HANGUP_WAITING_OR_BACKGROUND instead of
24 RIL_REQUEST_HANGUP for incoming calls.
1125
12 -- tony espy <espy@canonical.com> Tue, 21 Jan 2014 13:54:46 -050026 -- Tony Espy <espy@canonical.com> Tue, 21 Jan 2014 13:54:46 -0500
1327
14ofono (1.12+bzr6851-0ubuntu1) trusty; urgency=low28ofono (1.12+bzr6851-0ubuntu1) trusty; urgency=low
1529
1630
=== modified file 'drivers/rilmodem/sim.c'
--- drivers/rilmodem/sim.c 2013-12-09 17:09:20 +0000
+++ drivers/rilmodem/sim.c 2014-02-03 22:25:52 +0000
@@ -245,7 +245,12 @@
245 if ((reply = g_ril_reply_parse_sim_io(sd->ril, message))245 if ((reply = g_ril_reply_parse_sim_io(sd->ril, message))
246 == NULL) {246 == NULL) {
247 ofono_error("Can't parse SIM IO response from RILD");247 ofono_error("Can't parse SIM IO response from RILD");
248 decode_ril_error(&error, "FAIL");248 goto error;
249 }
250
251 if (reply->hex_len == 0) {
252 ofono_error("Null SIM IO response from RILD");
253 g_ril_reply_free_sim_io(reply);
249 goto error;254 goto error;
250 }255 }
251256
252257
=== modified file 'drivers/rilmodem/voicecall.c'
--- drivers/rilmodem/voicecall.c 2014-01-02 08:53:29 +0000
+++ drivers/rilmodem/voicecall.c 2014-02-03 22:25:52 +0000
@@ -433,13 +433,28 @@
433433
434 for (l = vd->calls; l; l = l->next) {434 for (l = vd->calls; l; l = l->next) {
435 call = l->data;435 call = l->data;
436 /* TODO: Hangup just the active ones once we have call436
437 * state tracking (otherwise it can't handle ringing) */437 if (call->status == CALL_STATUS_INCOMING) {
438 g_ril_request_hangup(vd->ril, call->id, &rilp);438 /*
439439 * Need to use this request so that declined
440 /* Send request to RIL */440 * calls in this state, are properly forwarded
441 ril_template(RIL_REQUEST_HANGUP, vc, generic_cb,441 * to voicemail. REQUEST_HANGUP doesn't do the
442 AFFECTED_STATES_ALL, &rilp, NULL, NULL);442 * right thing for some operators, causing the
443 * caller to hear a fast busy signal.
444 */
445 ril_template(RIL_REQUEST_HANGUP_WAITING_OR_BACKGROUND,
446 vc, generic_cb, AFFECTED_STATES_ALL,
447 NULL, NULL, NULL);
448 } else {
449
450 /* TODO: Hangup just the active ones once we have call
451 * state tracking (otherwise it can't handle ringing) */
452 g_ril_request_hangup(vd->ril, call->id, &rilp);
453
454 /* Send request to RIL */
455 ril_template(RIL_REQUEST_HANGUP, vc, generic_cb,
456 AFFECTED_STATES_ALL, &rilp, NULL, NULL);
457 }
443 }458 }
444459
445 /* TODO: Deal in case of an error at hungup */460 /* TODO: Deal in case of an error at hungup */
446461
=== modified file 'gril/grilreply.c'
--- gril/grilreply.c 2013-12-10 09:44:10 +0000
+++ gril/grilreply.c 2014-02-03 22:25:52 +0000
@@ -685,23 +685,26 @@
685685
686 response = parcel_r_string(&rilp);686 response = parcel_r_string(&rilp);
687687
688 g_ril_append_print_buf(gril,
689 "(sw1=0x%.2X,sw2=0x%.2X,%s)",
690 reply->sw1,
691 reply->sw2,
692 response);
693 g_ril_print_response(gril, message);
694
695 if (rilp.malformed)
696 goto error;
697
688 if (response != NULL) {698 if (response != NULL) {
689 reply->hex_response =699 reply->hex_response =
690 decode_hex(response, strlen(response),700 decode_hex(response, strlen(response),
691 (long *) &reply->hex_len, -1);701 (long *) &reply->hex_len, -1);
702 g_free(response);
703
692 if (reply->hex_response == NULL)704 if (reply->hex_response == NULL)
693 goto error;705 goto error;
694 }706 }
695707
696 g_ril_append_print_buf(gril,
697 "(sw1=0x%.2X,sw2=0x%.2X,%s)",
698 reply->sw1,
699 reply->sw2,
700 response);
701 g_ril_print_response(gril, message);
702
703 g_free(response);
704
705 return reply;708 return reply;
706709
707error:710error:
708711
=== modified file 'plugins/ril.c'
--- plugins/ril.c 2014-01-08 08:32:56 +0000
+++ plugins/ril.c 2014-02-03 22:25:52 +0000
@@ -257,6 +257,7 @@
257 struct ril_data *ril = ofono_modem_get_data(modem);257 struct ril_data *ril = ofono_modem_get_data(modem);
258 struct ofono_gprs *gprs;258 struct ofono_gprs *gprs;
259 struct ofono_gprs_context *gc;259 struct ofono_gprs_context *gc;
260 struct ofono_message_waiting *mw;
260261
261 /* TODO: this function should setup:262 /* TODO: this function should setup:
262 * - phonebook263 * - phonebook
@@ -272,6 +273,10 @@
272 DBG("calling gprs_add_context");273 DBG("calling gprs_add_context");
273 ofono_gprs_add_context(gprs, gc);274 ofono_gprs_add_context(gprs, gc);
274 }275 }
276
277 mw = ofono_message_waiting_create(modem);
278 if (mw)
279 ofono_message_waiting_register(mw);
275}280}
276281
277static void ril_post_online(struct ofono_modem *modem)282static void ril_post_online(struct ofono_modem *modem)
278283
=== modified file 'src/smsutil.c'
--- src/smsutil.c 2012-10-01 14:52:32 +0000
+++ src/smsutil.c 2014-02-03 22:25:52 +0000
@@ -240,7 +240,7 @@
240 else240 else
241 ch = SMS_CHARSET_7BIT;241 ch = SMS_CHARSET_7BIT;
242242
243 act = dcs & 0x8;243 act = (dcs & 0x8) ? TRUE : FALSE;
244244
245 t = (enum sms_mwi_type) (dcs & 0x3);245 t = (enum sms_mwi_type) (dcs & 0x3);
246246
247247
=== modified file 'unit/test-grilreply.c'
--- unit/test-grilreply.c 2014-01-21 19:35:47 +0000
+++ unit/test-grilreply.c 2014-02-03 22:25:52 +0000
@@ -1128,6 +1128,74 @@
1128};1128};
11291129
1130/*1130/*
1131 * The following hexadecimal data contains the event data of an valid
1132 * RIL_REQUEST_SIM_IO reply with the following parameters:
1133 *
1134 * {sw1=0x90,sw2=0x00,(null)}
1135 * This is a reply to a select file for EF_ICCID.
1136 */
1137static const guchar reply_sim_io_valid_parcel2[] = {
1138 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff
1139};
1140
1141static const struct ril_msg reply_sim_io_valid_2 = {
1142 .buf = (gchar *) reply_sim_io_valid_parcel2,
1143 .buf_len = sizeof(reply_sim_io_valid_parcel2),
1144 .unsolicited = FALSE,
1145 .req = RIL_REQUEST_SIM_IO,
1146 .serial_no = 0,
1147 .error = 0,
1148};
1149
1150/*
1151 * The following hexadecimal data contains the event data of an invalid
1152 * RIL_REQUEST_SIM_IO reply with the following parameters:
1153 *
1154 * Note - this is invalid because the response includes a non-hex char ('Z').
1155 *
1156 * {sw1=0x90,sw2=0x00,Z000000a2fe2040000000005020000}
1157 * This is a reply to a select file for EF_ICCID.
1158 */
1159static const guchar reply_sim_io_invalid_parcel1[] = {
1160 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1e, 0x00, 0x00, 0x00,
1161 0x5A, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
1162 0x30, 0x00, 0x61, 0x00, 0x32, 0x00, 0x66, 0x00, 0x65, 0x00, 0x32, 0x00,
1163 0x30, 0x00, 0x34, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
1164 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x35, 0x00,
1165 0x30, 0x00, 0x32, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
1166 0x00, 0x00, 0x00, 0x00
1167};
1168
1169static const struct ril_msg reply_sim_io_invalid_1 = {
1170 .buf = (gchar *) reply_sim_io_invalid_parcel1,
1171 .buf_len = sizeof(reply_sim_io_invalid_parcel1),
1172 .unsolicited = FALSE,
1173 .req = RIL_REQUEST_SIM_IO,
1174 .serial_no = 0,
1175 .error = 0,
1176};
1177
1178/*
1179 * The following hexadecimal data contains the event data of an invalid
1180 * RIL_REQUEST_SIM_IO reply with the following parameters:
1181 *
1182 * {sw1=0x90,sw2=0x00,<malformed length>}
1183 * This is a reply to a select file for EF_ICCID.
1184 */
1185static const guchar reply_sim_io_invalid_parcel2[] = {
1186 0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff
1187};
1188
1189static const struct ril_msg reply_sim_io_invalid_2 = {
1190 .buf = (gchar *) reply_sim_io_invalid_parcel2,
1191 .buf_len = sizeof(reply_sim_io_invalid_parcel2),
1192 .unsolicited = FALSE,
1193 .req = RIL_REQUEST_SIM_IO,
1194 .serial_no = 0,
1195 .error = 0,
1196};
1197
1198/*
1131 * The following hexadecimal data contains the event data of a valid1199 * The following hexadecimal data contains the event data of a valid
1132 * RIL_REQUEST_GET_IMSI reply with the following parameters:1200 * RIL_REQUEST_GET_IMSI reply with the following parameters:
1133 *1201 *
@@ -1485,6 +1553,12 @@
1485 g_ril_reply_free_sim_io(reply);1553 g_ril_reply_free_sim_io(reply);
1486}1554}
14871555
1556static void test_reply_sim_io_invalid(gconstpointer data)
1557{
1558 struct reply_sim_io *reply = g_ril_reply_parse_sim_io(NULL, data);
1559 g_assert(reply == NULL);
1560}
1561
1488static void test_reply_imsi_valid(gconstpointer data)1562static void test_reply_imsi_valid(gconstpointer data)
1489{1563{
1490 gchar *reply = g_ril_reply_parse_imsi(NULL, data);1564 gchar *reply = g_ril_reply_parse_imsi(NULL, data);
@@ -1790,6 +1864,21 @@
1790 test_reply_sim_io_valid);1864 test_reply_sim_io_valid);
17911865
1792 g_test_add_data_func("/testgrilreply/sim: "1866 g_test_add_data_func("/testgrilreply/sim: "
1867 "valid SIM_IO Test 2",
1868 &reply_sim_io_valid_2,
1869 test_reply_sim_io_valid);
1870
1871 g_test_add_data_func("/testgrilreply/sim: "
1872 "invalid SIM_IO Test 1",
1873 &reply_sim_io_invalid_1,
1874 test_reply_sim_io_invalid);
1875
1876 g_test_add_data_func("/testgrilreply/sim: "
1877 "invalid SIM_IO Test 2",
1878 &reply_sim_io_invalid_2,
1879 test_reply_sim_io_invalid);
1880
1881 g_test_add_data_func("/testgrilreply/sim: "
1793 "valid GET_IMSI Test 1",1882 "valid GET_IMSI Test 1",
1794 &reply_imsi_valid_1,1883 &reply_imsi_valid_1,
1795 test_reply_imsi_valid);1884 test_reply_imsi_valid);

Subscribers

People subscribed via source and target branches

to all changes: