[PATCH 0/9] Miscellaneous patches

[PATCH 0/9] Miscellaneous patches

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 977 bytes --]

This series is just some miscellaneous patches, addressing some compiler
warnings, etc.  The patches are independent of each other.

/Jonas

Jonas Bonn (9):
  test: use python3 for set-ddr
  mbim: remove unused modem data
  atmodem: enlarge command buffer
  qmimodem: prevent use of unitialized variable
  simutil: remove check for impossible condition
  stkutil: remove test for impossible condition
  huaweimodme: prevent use of uninitialized variable
  atmodem: prevent use of uninitialized value
  modem: global data is pre-zeroed

 drivers/atmodem/sms.c      |  2 +-
 drivers/atmodem/ussd.c     |  6 +++---
 drivers/huaweimodem/ussd.c |  6 +++---
 drivers/qmimodem/lte.c     |  2 +-
 plugins/udevng.c           |  2 --
 src/modem.c                | 14 +++++++-------
 src/simutil.c              |  3 ---
 src/stkutil.c              |  3 ---
 test/set-ddr               |  2 +-
 9 files changed, 16 insertions(+), 24 deletions(-)

-- 
2.17.1

[PATCH 1/9] test: use python3 for set-ddr

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

---
 test/set-ddr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/set-ddr b/test/set-ddr
index 5d061b95..33631f31 100755
--- a/test/set-ddr
+++ b/test/set-ddr
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 
 import sys
 import dbus
-- 
2.17.1

[PATCH 2/9] mbim: remove unused modem data

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 700 bytes --]

Neither the Vendor nor Model strings are used by the mbim plugin.
---
 plugins/udevng.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 3c39e681..11338f78 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -962,8 +962,6 @@ static gboolean setup_mbim(struct modem_info *modem)
 	ofono_modem_set_string(modem->modem, "Device", ctl);
 	ofono_modem_set_string(modem->modem, "NetworkInterface", net);
 	ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
-	ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
-	ofono_modem_set_string(modem->modem, "Model", modem->model);
 
 	return TRUE;
 }
-- 
2.17.1

[PATCH 3/9] atmodem: enlarge command buffer

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

The ofono phone number max length is 80 so a buffer size of 64 is
obviously insufficient.  Expanding the buffer to 128 prevents a
potential failure and suppresses the folowing compiler warning:

../drivers/atmodem/sms.c: In function ‘at_csca_set’:
../drivers/atmodem/sms.c:108:40: warning: ‘%s’ directive output may be truncated writing up to 80 bytes into a region of size 55 [-Wformat-truncation=]
  snprintf(buf, sizeof(buf), "AT+CSCA=\"%s\",%d", sca->number, sca->type);
                                        ^~
../drivers/atmodem/sms.c:108:2: note: ‘snprintf’ output between 13 and 103 bytes into a destination of size 64
  snprintf(buf, sizeof(buf), "AT+CSCA=\"%s\",%d", sca->number, sca->type);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
---
 drivers/atmodem/sms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atmodem/sms.c b/drivers/atmodem/sms.c
index 380cd763..277d6517 100644
--- a/drivers/atmodem/sms.c
+++ b/drivers/atmodem/sms.c
@@ -103,7 +103,7 @@ static void at_csca_set(struct ofono_sms *sms,
 {
 	struct sms_data *data = ofono_sms_get_data(sms);
 	struct cb_data *cbd = cb_data_new(cb, user_data);
-	char buf[64];
+	char buf[128];
 
 	snprintf(buf, sizeof(buf), "AT+CSCA=\"%s\",%d", sca->number, sca->type);
 
-- 
2.17.1

[PATCH 4/9] qmimodem: prevent use of unitialized variable

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

From: Jonas Bonn <jonas@southpole.se>

'index' may not be initialized in the error path so don't try to print a
message with it.
---
 drivers/qmimodem/lte.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/qmimodem/lte.c b/drivers/qmimodem/lte.c
index e0333ecf..1337e7f9 100644
--- a/drivers/qmimodem/lte.c
+++ b/drivers/qmimodem/lte.c
@@ -164,7 +164,7 @@ static void get_default_profile_cb(struct qmi_result *result, void *user_data)
 	qmi_param_free(param);
 
 error:
-	ofono_error("Failed to reset profile %hhd", index);
+	ofono_error("Failed to reset default profile");
 	ofono_lte_remove(lte);
 }
 
-- 
2.17.1

[PATCH 5/9] simutil: remove check for impossible condition

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

From: Jonas Bonn <jonas@southpole.se>

Addresses following warning:

src/simutil.c:1546:12: error: comparison of constant 8 with expression
of type
      'enum sim_cphs_service' is always false
      [-Werror,-Wtautological-constant-out-of-range-compare]
        if (index >= 2 * 4u)
---
 src/simutil.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/simutil.c b/src/simutil.c
index 9287df17..7f0e316e 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -1543,9 +1543,6 @@ gboolean sim_sst_is_active(unsigned char *efsst, unsigned char len,
 
 gboolean sim_cphs_is_active(unsigned char *cphs, enum sim_cphs_service index)
 {
-	if (index >= 2 * 4u)
-		return FALSE;
-
 	return ((cphs[index / 4] >> ((index % 4) * 2)) & 3) == 3;
 }
 
-- 
2.17.1

[PATCH 6/9] stkutil: remove test for impossible condition

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 744 bytes --]

From: Jonas Bonn <jonas@southpole.se>

'string' is an array and therefore never NULL so this test always fails.
---
 src/stkutil.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/stkutil.c b/src/stkutil.c
index ec3f825d..9992f6c5 100644
--- a/src/stkutil.c
+++ b/src/stkutil.c
@@ -4405,9 +4405,6 @@ static gboolean build_dataobj_ussd_string(struct stk_tlv_builder *tlv,
 	const struct stk_ussd_string *ussd = data;
 	unsigned char tag = STK_DATA_OBJECT_TYPE_USSD_STRING;
 
-	if (ussd->string == NULL)
-		return TRUE;
-
 	return stk_tlv_builder_open_container(tlv, cr, tag, FALSE) &&
 		stk_tlv_builder_append_byte(tlv, ussd->dcs) &&
 		stk_tlv_builder_append_bytes(tlv, ussd->string, ussd->len) &&
-- 
2.17.1

[PATCH 7/9] huaweimodme: prevent use of uninitialized variable

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

From: Jonas Bonn <jonas@southpole.se>

Move initialization of 'dcs' ahead of 'content' fetch to prevent
uninitialized use.
---
 drivers/huaweimodem/ussd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/huaweimodem/ussd.c b/drivers/huaweimodem/ussd.c
index fbed3cd0..f4001108 100644
--- a/drivers/huaweimodem/ussd.c
+++ b/drivers/huaweimodem/ussd.c
@@ -62,12 +62,12 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
 	if (!g_at_result_iter_next_number(&iter, &status))
 		return;
 
-	if (!g_at_result_iter_next_string(&iter, &content))
-		goto out;
-
 	if (!g_at_result_iter_next_number(&iter, &dcs))
 		dcs = 0;
 
+	if (!g_at_result_iter_next_string(&iter, &content))
+		goto out;
+
 	msg_ptr = decode_hex_own_buf(content, -1, &msg_len, 0, msg);
 
 out:
-- 
2.17.1

[PATCH 8/9] atmodem: prevent use of uninitialized value

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

From: Jonas Bonn <jonas@southpole.se>

Move initialization of 'dcs' ahead of 'content' fetch to prevent using
uninitialized 'dcs' in ofono_ussd_notify.
---
 drivers/atmodem/ussd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
index f1907a00..8416bec6 100644
--- a/drivers/atmodem/ussd.c
+++ b/drivers/atmodem/ussd.c
@@ -117,12 +117,12 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
 	if (!g_at_result_iter_next_number(&iter, &status))
 		return;
 
-	if (!g_at_result_iter_next_string(&iter, &content))
-		goto out;
-
 	if (!g_at_result_iter_next_number(&iter, &dcs))
 		dcs = 0;
 
+	if (!g_at_result_iter_next_string(&iter, &content))
+		goto out;
+
 	if (!cbs_dcs_decode(dcs, NULL, NULL, &charset, NULL, NULL, NULL)) {
 		ofono_error("Unsupported USSD data coding scheme (%02x)", dcs);
 		status = 4; /* Not supported */
-- 
2.17.1

[PATCH 9/9] modem: global data is pre-zeroed

[Jonas Bonn]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

Module-local and global data are BSS symbols and don't require
zero-initialization.
---
 src/modem.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 838967bf..bc462751 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -36,15 +36,15 @@
 
 #define DEFAULT_POWERED_TIMEOUT (20)
 
-static GSList *g_devinfo_drivers = NULL;
-static GSList *g_driver_list = NULL;
-static GSList *g_modem_list = NULL;
+static GSList *g_devinfo_drivers;
+static GSList *g_driver_list;
+static GSList *g_modem_list;
 
-static int next_modem_id = 0;
-static gboolean powering_down = FALSE;
-static int modems_remaining = 0;
+static int next_modem_id;
+static gboolean powering_down;
+static int modems_remaining;
 
-static struct ofono_watchlist *g_modemwatches = NULL;
+static struct ofono_watchlist *g_modemwatches;
 
 enum property_type {
 	PROPERTY_TYPE_INVALID = 0,
-- 
2.17.1

Re: [PATCH 7/9] huaweimodme: prevent use of uninitialized variable

[Giacinto Cifelli]

[-- Attachment #1: Type: text/plain, Size: 1186 bytes --]

hi Jonas,

On Fri, 26 Oct 2018, 12:13 Jonas Bonn, <jonas@norrbonn.se> wrote:

> From: Jonas Bonn <jonas@southpole.se>
>
> Move initialization of 'dcs' ahead of 'content' fetch to prevent
> uninitialized use.
> ---
>  drivers/huaweimodem/ussd.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/huaweimodem/ussd.c b/drivers/huaweimodem/ussd.c
> index fbed3cd0..f4001108 100644
> --- a/drivers/huaweimodem/ussd.c
> +++ b/drivers/huaweimodem/ussd.c
> @@ -62,12 +62,12 @@ static void cusd_parse(GAtResult *result, struct
> ofono_ussd *ussd)
>         if (!g_at_result_iter_next_number(&iter, &status))
>                 return;
>
> -       if (!g_at_result_iter_next_string(&iter, &content))
> -               goto out;
> -
>         if (!g_at_result_iter_next_number(&iter, &dcs))
>                 dcs = 0;
>
> +       if (!g_at_result_iter_next_string(&iter, &content))
> +               goto out;
> +
>

aren't you changing the order of the parameters in the command response?
Have you tested this on an actual modem?

        msg_ptr = decode_hex_own_buf(content, -1, &msg_len, 0, msg);
>
> regards,
Giacinto

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1972 bytes --]

Re: [PATCH 1/9] test: use python3 for set-ddr

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 197 bytes --]

Hi Jonas,

On 10/26/2018 05:13 AM, Jonas Bonn wrote:
> ---
>   test/set-ddr | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 

Patches 1-4 applied, thanks.

Regards,
-Denis

Re: [PATCH 5/9] simutil: remove check for impossible condition

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Hi Jonas,

On 10/26/2018 05:13 AM, Jonas Bonn wrote:
> From: Jonas Bonn <jonas@southpole.se>
> 
> Addresses following warning:
> 
> src/simutil.c:1546:12: error: comparison of constant 8 with expression
> of type
>        'enum sim_cphs_service' is always false
>        [-Werror,-Wtautological-constant-out-of-range-compare]
>          if (index >= 2 * 4u)

This is clang right?

> ---
>   src/simutil.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/src/simutil.c b/src/simutil.c
> index 9287df17..7f0e316e 100644
> --- a/src/simutil.c
> +++ b/src/simutil.c
> @@ -1543,9 +1543,6 @@ gboolean sim_sst_is_active(unsigned char *efsst, unsigned char len,
>   
>   gboolean sim_cphs_is_active(unsigned char *cphs, enum sim_cphs_service index)
>   {
> -	if (index >= 2 * 4u)
> -		return FALSE;
> -

This is where gcc & clang give differing advice.  In theory there's 
nothing preventing index from being any value up to 
MAX_UCHAR/MAX_USHRT/MAX_UINT depending on what the compiler chooses to 
represent enums as.

>   	return ((cphs[index / 4] >> ((index % 4) * 2)) & 3) == 3;
>   }
>   
> 

Regards,
-Denis

Re: [PATCH 6/9] stkutil: remove test for impossible condition

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

Hi Jonas,

On 10/26/2018 05:13 AM, Jonas Bonn wrote:
> From: Jonas Bonn <jonas@southpole.se>
> 
> 'string' is an array and therefore never NULL so this test always fails.
> ---
>   src/stkutil.c | 3 ---
>   1 file changed, 3 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH 8/9] atmodem: prevent use of uninitialized value

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

Hi Jonas,

On 10/26/2018 05:13 AM, Jonas Bonn wrote:
> From: Jonas Bonn <jonas@southpole.se>
> 
> Move initialization of 'dcs' ahead of 'content' fetch to prevent using
> uninitialized 'dcs' in ofono_ussd_notify.
> ---
>   drivers/atmodem/ussd.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
> index f1907a00..8416bec6 100644
> --- a/drivers/atmodem/ussd.c
> +++ b/drivers/atmodem/ussd.c
> @@ -117,12 +117,12 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
>   	if (!g_at_result_iter_next_number(&iter, &status))
>   		return;
>   
> -	if (!g_at_result_iter_next_string(&iter, &content))
> -		goto out;
> -

This looks wrong.  Maybe just initialize DCS to 0 here

>   	if (!g_at_result_iter_next_number(&iter, &dcs))
>   		dcs = 0;
>   
> +	if (!g_at_result_iter_next_string(&iter, &content))
> +		goto out;
> +


>   	if (!cbs_dcs_decode(dcs, NULL, NULL, &charset, NULL, NULL, NULL)) {
>   		ofono_error("Unsupported USSD data coding scheme (%02x)", dcs);
>   		status = 4; /* Not supported */
> 

Regards,
-Denis

Re: [PATCH 9/9] modem: global data is pre-zeroed

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

Hi Jonas,

On 10/26/2018 05:13 AM, Jonas Bonn wrote:
> Module-local and global data are BSS symbols and don't require
> zero-initialization.
> ---
>   src/modem.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)

Applied, thanks.

Regards,
-Denis