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

[Tony Espy]

* 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.

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.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:6854
http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-ci/22/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-armhf-ci/15
        deb: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-armhf-ci/15/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-i386-ci/15

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-team-ofono-ubuntu-ci/22/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:6853
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~phablet-team/ofono/ofono-mwi-and-bugs/+merge/204587/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-ci/24/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-amd64-ci/17
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-armhf-ci/17
        deb: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-armhf-ci/17/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-ofono-ubuntu-trusty-i386-ci/17

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-team-ofono-ubuntu-ci/24/rebuild

review: Needs Fixing (continuous-integration)

=== 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 @@
 ofono (1.12+bzr6851-0ubuntu2) UNRELEASED; urgency=low
+
   [ Tony Espy ]
   * unit: fix fail-to-build on powerpc
     The previously released version enabled -Wall,
@@ -8,8 +9,21 @@
     systems via an ifdef, which leaves unused functions
     and data.  This change exends the ifdef to cover
     the unused functions and test data.
+   * 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.
 
- -- tony espy <[e-mail]>  Tue, 21 Jan 2014 13:54:46 -0500
+ -- Tony Espy <[e-mail]>  Tue, 21 Jan 2014 13:54:46 -0500
 
 ofono (1.12+bzr6851-0ubuntu1) trusty; urgency=low
 

=== 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 @@
 	if ((reply = g_ril_reply_parse_sim_io(sd->ril, message))
 			== NULL) {
 		ofono_error("Can't parse SIM IO response from RILD");
-		decode_ril_error(&error, "FAIL");
+		goto error;
+	}
+
+	if (reply->hex_len == 0) {
+		ofono_error("Null SIM IO response from RILD");
+		g_ril_reply_free_sim_io(reply);
 		goto error;
 	}
 

=== 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 @@
 
 	for (l = vd->calls; l; l = l->next) {
 		call = l->data;
-		/* TODO: Hangup just the active ones once we have call
-		 * state tracking (otherwise it can't handle ringing) */
-		g_ril_request_hangup(vd->ril, call->id, &rilp);
-
-		/* Send request to RIL */
-		ril_template(RIL_REQUEST_HANGUP, vc, generic_cb,
-				AFFECTED_STATES_ALL, &rilp, NULL, NULL);
+
+		if (call->status == CALL_STATUS_INCOMING) {
+			/*
+			 * Need to use this request so that declined
+			 * calls in this state, are properly forwarded
+			 * to voicemail.  REQUEST_HANGUP doesn't do the
+			 * right thing for some operators, causing the
+			 * caller to hear a fast busy signal.
+			 */
+			ril_template(RIL_REQUEST_HANGUP_WAITING_OR_BACKGROUND,
+					vc, generic_cb, AFFECTED_STATES_ALL,
+					NULL, NULL, NULL);
+		} else {
+
+			/* TODO: Hangup just the active ones once we have call
+			 * state tracking (otherwise it can't handle ringing) */
+			g_ril_request_hangup(vd->ril, call->id, &rilp);
+
+			/* Send request to RIL */
+			ril_template(RIL_REQUEST_HANGUP, vc, generic_cb,
+					AFFECTED_STATES_ALL, &rilp, NULL, NULL);
+		}
 	}
 
 	/* TODO: Deal in case of an error at hungup */

=== 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 @@
 
 	response = parcel_r_string(&rilp);
 
+	g_ril_append_print_buf(gril,
+				"(sw1=0x%.2X,sw2=0x%.2X,%s)",
+				reply->sw1,
+				reply->sw2,
+				response);
+	g_ril_print_response(gril, message);
+
+	if (rilp.malformed)
+		goto error;
+
 	if (response != NULL) {
 		reply->hex_response =
 			decode_hex(response, strlen(response),
 					(long *) &reply->hex_len, -1);
+		g_free(response);
+
 		if (reply->hex_response == NULL)
 			goto error;
 	}
 
-	g_ril_append_print_buf(gril,
-				"(sw1=0x%.2X,sw2=0x%.2X,%s)",
-				reply->sw1,
-				reply->sw2,
-				response);
-	g_ril_print_response(gril, message);
-
-	g_free(response);
-
 	return reply;
 
 error:

=== 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 @@
 	struct ril_data *ril = ofono_modem_get_data(modem);
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
+	struct ofono_message_waiting *mw;
 
 	/* TODO: this function should setup:
 	 *  - phonebook
@@ -272,6 +273,10 @@
 		DBG("calling gprs_add_context");
 		ofono_gprs_add_context(gprs, gc);
 	}
+
+	mw = ofono_message_waiting_create(modem);
+	if (mw)
+		ofono_message_waiting_register(mw);
 }
 
 static void ril_post_online(struct ofono_modem *modem)

=== 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 @@
 	else
 		ch = SMS_CHARSET_7BIT;
 
-	act = dcs & 0x8;
+	act = (dcs & 0x8) ? TRUE : FALSE;
 
 	t = (enum sms_mwi_type) (dcs & 0x3);
 

=== 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 @@
 };
 
 /*
+ * The following hexadecimal data contains the event data of an valid
+ * RIL_REQUEST_SIM_IO reply with the following parameters:
+ *
+ * {sw1=0x90,sw2=0x00,(null)}
+ * This is a reply to a select file for EF_ICCID.
+ */
+static const guchar reply_sim_io_valid_parcel2[] = {
+	0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff
+};
+
+static const struct ril_msg reply_sim_io_valid_2 = {
+	.buf = (gchar *) reply_sim_io_valid_parcel2,
+	.buf_len = sizeof(reply_sim_io_valid_parcel2),
+	.unsolicited = FALSE,
+	.req = RIL_REQUEST_SIM_IO,
+	.serial_no = 0,
+	.error = 0,
+};
+
+/*
+ * The following hexadecimal data contains the event data of an invalid
+ * RIL_REQUEST_SIM_IO reply with the following parameters:
+ *
+ * Note - this is invalid because the response includes a non-hex char ('Z').
+ *
+ * {sw1=0x90,sw2=0x00,Z000000a2fe2040000000005020000}
+ * This is a reply to a select file for EF_ICCID.
+ */
+static const guchar reply_sim_io_invalid_parcel1[] = {
+	0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1e, 0x00, 0x00, 0x00,
+	0x5A, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
+	0x30, 0x00, 0x61, 0x00, 0x32, 0x00, 0x66, 0x00, 0x65, 0x00, 0x32, 0x00,
+	0x30, 0x00, 0x34, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
+	0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x35, 0x00,
+	0x30, 0x00, 0x32, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00, 0x30, 0x00,
+	0x00, 0x00, 0x00, 0x00
+};
+
+static const struct ril_msg reply_sim_io_invalid_1 = {
+	.buf = (gchar *) reply_sim_io_invalid_parcel1,
+	.buf_len = sizeof(reply_sim_io_invalid_parcel1),
+	.unsolicited = FALSE,
+	.req = RIL_REQUEST_SIM_IO,
+	.serial_no = 0,
+	.error = 0,
+};
+
+/*
+ * The following hexadecimal data contains the event data of an invalid
+ * RIL_REQUEST_SIM_IO reply with the following parameters:
+ *
+ * {sw1=0x90,sw2=0x00,<malformed length>}
+ * This is a reply to a select file for EF_ICCID.
+ */
+static const guchar reply_sim_io_invalid_parcel2[] = {
+	0x90, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff
+};
+
+static const struct ril_msg reply_sim_io_invalid_2 = {
+	.buf = (gchar *) reply_sim_io_invalid_parcel2,
+	.buf_len = sizeof(reply_sim_io_invalid_parcel2),
+	.unsolicited = FALSE,
+	.req = RIL_REQUEST_SIM_IO,
+	.serial_no = 0,
+	.error = 0,
+};
+
+/*
  * The following hexadecimal data contains the event data of a valid
  * RIL_REQUEST_GET_IMSI reply with the following parameters:
  *
@@ -1485,6 +1553,12 @@
 	g_ril_reply_free_sim_io(reply);
 }
 
+static void test_reply_sim_io_invalid(gconstpointer data)
+{
+	struct reply_sim_io *reply = g_ril_reply_parse_sim_io(NULL, data);
+	g_assert(reply == NULL);
+}
+
 static void test_reply_imsi_valid(gconstpointer data)
 {
 	gchar *reply = g_ril_reply_parse_imsi(NULL, data);
@@ -1790,6 +1864,21 @@
 				test_reply_sim_io_valid);
 
 	g_test_add_data_func("/testgrilreply/sim: "
+				"valid SIM_IO Test 2",
+				&reply_sim_io_valid_2,
+				test_reply_sim_io_valid);
+
+	g_test_add_data_func("/testgrilreply/sim: "
+				"invalid SIM_IO Test 1",
+				&reply_sim_io_invalid_1,
+				test_reply_sim_io_invalid);
+
+	g_test_add_data_func("/testgrilreply/sim: "
+				"invalid SIM_IO Test 2",
+				&reply_sim_io_invalid_2,
+				test_reply_sim_io_invalid);
+
+	g_test_add_data_func("/testgrilreply/sim: "
 				"valid GET_IMSI Test 1",
 				&reply_imsi_valid_1,
 				test_reply_imsi_valid);