9:13 p.m.
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
9:13 p.m.
New subject: [PATCH 1/9] test: use python3 for set-ddr
---
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
9:13 p.m.
New subject: [PATCH 2/9] mbim: remove unused modem data
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
9:13 p.m.
New subject: [PATCH 3/9] atmodem: enlarge command buffer
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
9:13 p.m.
New subject: [PATCH 4/9] qmimodem: prevent use of unitialized variable
From: Jonas Bonn <jonas(a)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
9:13 p.m.
New subject: [PATCH 5/9] simutil: remove check for impossible condition
From: Jonas Bonn <jonas(a)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
5:44 a.m.
New subject: [PATCH 5/9] simutil: remove check for impossible condition
Hi Jonas,
On 10/26/2018 05:13 AM, Jonas Bonn wrote:
From: Jonas Bonn <jonas(a)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
9:13 p.m.
New subject: [PATCH 6/9] stkutil: remove test for impossible condition
From: Jonas Bonn <jonas(a)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
5:59 a.m.
New subject: [PATCH 6/9] stkutil: remove test for impossible condition
Hi Jonas,
On 10/26/2018 05:13 AM, Jonas Bonn wrote:
From: Jonas Bonn <jonas(a)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
9:13 p.m.
New subject: [PATCH 7/9] huaweimodme: prevent use of uninitialized variable
From: Jonas Bonn <jonas(a)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
9:32 p.m.
New subject: [PATCH 7/9] huaweimodme: prevent use of uninitialized variable
hi Jonas,
On Fri, 26 Oct 2018, 12:13 Jonas Bonn, <jonas(a)norrbonn.se> wrote:
From: Jonas Bonn <jonas(a)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
9:13 p.m.
New subject: [PATCH 8/9] atmodem: prevent use of uninitialized value
From: Jonas Bonn <jonas(a)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
6:04 a.m.
New subject: [PATCH 8/9] atmodem: prevent use of uninitialized value
Hi Jonas,
On 10/26/2018 05:13 AM, Jonas Bonn wrote:
From: Jonas Bonn <jonas(a)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
9:13 p.m.
New subject: [PATCH 9/9] modem: global data is pre-zeroed
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
6:05 a.m.
New subject: [PATCH 9/9] modem: global data is pre-zeroed
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
1420
days inactive
1423
days old
15 comments
3 participants
participants (3)
-
Denis Kenzior -
Giacinto Cifelli -
Jonas Bonn