[PATCH 0/9] Miscellaneous patches

[Jonas Bonn]

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(-)

[Jonas Bonn]

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

[Denis Kenzior]

Hi Jonas,

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

Patches 1-4 applied, thanks.

Regards,
-Denis

[Jonas Bonn]

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;
}

[Jonas Bonn]

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);

[Jonas Bonn]

From: Jonas Bonn <***@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);
}

[Jonas Bonn]

From: Jonas Bonn <***@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;
}

[Denis Kenzior]

Hi Jonas,

Post by Jonas Bonn
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?

Post by Jonas Bonn
---
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.

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

Regards,
-Denis

[Jonas Bonn]

From: Jonas Bonn <***@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) &&

[Denis Kenzior]

Hi Jonas,

Post by Jonas Bonn
'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

[Jonas Bonn]

From: Jonas Bonn <***@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:

[Jonas Bonn]

From: Jonas Bonn <***@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 */

[Denis Kenzior]

Hi Jonas,

Post by Jonas Bonn
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

Post by Jonas Bonn
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

[Jonas Bonn]

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,

[Denis Kenzior]

Hi Jonas,

Post by Jonas Bonn
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