[v2,01/14] simutil: Convert eons APIs to use ell

Commit Message

Denis Kenzior April 3, 2024, 4:05 p.m. UTC

While here, also remove sim_eons_optimize() as it is no longer needed.
The ell queue implementation natively supports pushing to the back of a
linked list.  Also tighten up some length checks and make 'length'
arguments unsigned.
---
 src/network.c       |   2 -
 src/simutil.c       | 116 +++++++++++++++++++++-----------------------
 src/simutil.h       |  15 +++---
 unit/test-simutil.c |   1 -
 4 files changed, 63 insertions(+), 71 deletions(-)

Comments

[e-mail] April 3, 2024, 5 p.m. UTC | #1

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <[e-mail]>:

On Wed,  3 Apr 2024 11:05:24 -0500 you wrote:
> While here, also remove sim_eons_optimize() as it is no longer needed.
> The ell queue implementation natively supports pushing to the back of a
> linked list.  Also tighten up some length checks and make 'length'
> arguments unsigned.
> ---
>  src/network.c       |   2 -
>  src/simutil.c       | 116 +++++++++++++++++++++-----------------------
>  src/simutil.h       |  15 +++---
>  unit/test-simutil.c |   1 -
>  4 files changed, 63 insertions(+), 71 deletions(-)

Here is the summary with links:
  - [v2,01/14] simutil: Convert eons APIs to use ell
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=9850e63ef1da
  - [v2,02/14] sim: Simplify SPN management logic
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=4a6c0786bea2
  - [v2,03/14] data: Remove AweSIM entry
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=ec4946390408
  - [v2,04/14] data: Mark some MVNOs via the "spn" attribute
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=b37e70242b4e
  - [v2,05/14] data: Remove outdated T-mobile settings
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=5594561c1f4f
  - [v2,06/14] data: Remove RESELLER settings from AT&T
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=c496380af19e
  - [v2,07/14] provisiontool: Add support for context tags
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=407309cf47f0
  - [v2,08/14] provisiondb: Add tags_filter support
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=61fbfea7e46d
  - [v2,09/14] tools: lookup-apn: add support for optional tags filter
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=bf76c826774f
  - [v2,10/14] data: Add tags for AT&T and T-Mobile contexts
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=5072741ab979
  - [v2,11/14] ofono: Add support for ofono main.conf settings
    (no matching commit)
  - [v2,12/14] provision: Add support for provision filter tags
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=00cc10773680
  - [v2,13/14] examples: Fix use after free and resource leaks
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=64e2380f4b37
  - [v2,14/14] unit: Add test cases with tags_filter provided
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=393b6cc3dcbe

You are awesome, thank you!

Patch

diff --git a/src/network.c b/src/network.c
index 886e5bf0f9a3..629d5fb91eb8 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1581,8 +1581,6 @@  static void sim_opl_read_cb(int ok, int length, int record,
 		return;
 
 optimize:
-	sim_eons_optimize(netreg->eons);
-
 	for (l = netreg->operator_list; l; l = l->next) {
 		struct network_operator_data *opd = l->data;
 		const struct sim_eons_operator_info *eons_info;
diff --git a/src/simutil.c b/src/simutil.c
index 543ab9962ef2..fbecf6db67de 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -36,10 +36,10 @@ 
 #include "missing.h"
 
 struct sim_eons {
-	struct sim_eons_operator_info *pnn_list;
-	GSList *opl_list;
-	gboolean pnn_valid;
-	int pnn_max;
+	struct l_queue *opl_list;
+	bool pnn_valid;
+	uint32_t pnn_max;
+	struct sim_eons_operator_info pnn_list[];
 };
 
 struct spdi_operator {
@@ -782,12 +782,12 @@  bool validate_utf8_tlv(const unsigned char *tlv)
 }
 
 static char *sim_network_name_parse(const unsigned char *buffer, int length,
-					gboolean *add_ci)
+					bool *add_ci)
 {
 	char *ret = NULL;
 	unsigned char dcs;
 	int i;
-	gboolean ci = FALSE;
+	bool ci = FALSE;
 	unsigned char *unpacked_buf;
 	long num_char, written;
 	int spare_bits;
@@ -999,22 +999,18 @@  void sim_spdi_free(struct sim_spdi *spdi)
 	g_free(spdi);
 }
 
-static void pnn_operator_free(struct sim_eons_operator_info *oper)
+struct sim_eons *sim_eons_new(uint32_t pnn_records)
 {
-	if (oper == NULL)
-		return;
+	struct sim_eons *eons = l_malloc(sizeof(struct sim_eons) +
+			sizeof(struct sim_eons_operator_info) * pnn_records);
 
-	l_free(oper->info);
-	l_free(oper->shortname);
-	l_free(oper->longname);
-}
+	eons->pnn_valid = false;
+	eons->pnn_max = pnn_records;
 
-struct sim_eons *sim_eons_new(int pnn_records)
-{
-	struct sim_eons *eons = g_new0(struct sim_eons, 1);
+	memset(eons->pnn_list, 0,
+			sizeof(struct sim_eons_operator_info) * pnn_records);
 
-	eons->pnn_list = g_new0(struct sim_eons_operator_info, pnn_records);
-	eons->pnn_max = pnn_records;
+	eons->opl_list = l_queue_new();
 
 	return eons;
 }
@@ -1024,8 +1020,8 @@  gboolean sim_eons_pnn_is_empty(struct sim_eons *eons)
 	return !eons->pnn_valid;
 }
 
-void sim_eons_add_pnn_record(struct sim_eons *eons, int record,
-				const guint8 *tlv, int length)
+void sim_eons_add_pnn_record(struct sim_eons *eons, uint32_t record,
+				const uint8_t *tlv, uint16_t length)
 {
 	const unsigned char *name;
 	int namelength;
@@ -1053,9 +1049,9 @@  void sim_eons_add_pnn_record(struct sim_eons *eons, int record,
 	eons->pnn_valid = TRUE;
 }
 
-static struct opl_operator *opl_operator_alloc(const guint8 *record)
+static struct opl_operator *opl_operator_alloc(const uint8_t *record)
 {
-	struct opl_operator *oper = g_new0(struct opl_operator, 1);
+	struct opl_operator *oper = l_new(struct opl_operator, 1);
 
 	sim_parse_mcc_mnc(record, oper->mcc, oper->mnc);
 	record += 3;
@@ -1071,40 +1067,52 @@  static struct opl_operator *opl_operator_alloc(const guint8 *record)
 }
 
 void sim_eons_add_opl_record(struct sim_eons *eons,
-				const guint8 *contents, int length)
+				const uint8_t *contents, uint16_t length)
 {
 	struct opl_operator *oper;
 
+	if (length < 8)
+		return;
+
 	oper = opl_operator_alloc(contents);
 
-	if (oper->id > eons->pnn_max) {
-		g_free(oper);
+	if (!oper->id || oper->id > eons->pnn_max) {
+		l_free(oper);
 		return;
 	}
 
-	eons->opl_list = g_slist_prepend(eons->opl_list, oper);
-}
-
-void sim_eons_optimize(struct sim_eons *eons)
-{
-	eons->opl_list = g_slist_reverse(eons->opl_list);
+	l_queue_push_tail(eons->opl_list, oper);
 }
 
 void sim_eons_free(struct sim_eons *eons)
 {
-	int i;
+	uint32_t i;
 
 	if (eons == NULL)
 		return;
 
-	for (i = 0; i < eons->pnn_max; i++)
-		pnn_operator_free(eons->pnn_list + i);
+	for (i = 0; i < eons->pnn_max; i++) {
+		struct sim_eons_operator_info *oper = eons->pnn_list + i;
 
-	g_free(eons->pnn_list);
+		l_free(oper->info);
+		l_free(oper->shortname);
+		l_free(oper->longname);
+	}
 
-	g_slist_free_full(eons->opl_list, g_free);
+	l_queue_destroy(eons->opl_list, l_free);
 
-	g_free(eons);
+	l_free(eons);
+}
+
+static bool opl_match_mcc_mnc(const char *opl, const char *s, size_t max)
+{
+	unsigned int i;
+
+	for (i = 0; i < max; i++)
+		if (s[i] != opl[i] && !(opl[i] == 'b' && s[i]))
+			return false;
+
+	return true;
 }
 
 static const struct sim_eons_operator_info *
@@ -1112,46 +1120,32 @@  static const struct sim_eons_operator_info *
 				const char *mcc, const char *mnc,
 				gboolean have_lac, guint16 lac)
 {
-	GSList *l;
+	const struct l_queue_entry *entry;
 	const struct opl_operator *opl;
-	int i;
 
-	for (l = eons->opl_list; l; l = l->next) {
-		opl = l->data;
+	for (entry = l_queue_get_entries(eons->opl_list);
+						entry; entry = entry->next) {
+		opl = entry->data;
 
-		for (i = 0; i < OFONO_MAX_MCC_LENGTH; i++)
-			if (mcc[i] != opl->mcc[i] &&
-					!(opl->mcc[i] == 'b' && mcc[i]))
-				break;
-		if (i < OFONO_MAX_MCC_LENGTH)
+		if (!opl_match_mcc_mnc(opl->mcc, mcc, OFONO_MAX_MCC_LENGTH))
 			continue;
 
-		for (i = 0; i < OFONO_MAX_MNC_LENGTH; i++)
-			if (mnc[i] != opl->mnc[i] &&
-					!(opl->mnc[i] == 'b' && mnc[i]))
-				break;
-		if (i < OFONO_MAX_MNC_LENGTH)
+		if (!opl_match_mcc_mnc(opl->mnc, mnc, OFONO_MAX_MNC_LENGTH))
 			continue;
 
 		if (opl->lac_tac_low == 0 && opl->lac_tac_high == 0xfffe)
-			break;
+			goto found;
 
 		if (have_lac == FALSE)
 			continue;
 
 		if ((lac >= opl->lac_tac_low) && (lac <= opl->lac_tac_high))
-			break;
+			goto found;
 	}
 
-	if (l == NULL)
-		return NULL;
-
-	opl = l->data;
-
-	/* 0 is not a valid record id */
-	if (opl->id == 0)
-		return NULL;
+	return NULL;
 
+found:
 	return &eons->pnn_list[opl->id - 1];
 }
 
diff --git a/src/simutil.h b/src/simutil.h
index 9584d4d9cd62..8c34f9f36488 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -20,6 +20,7 @@ 
  */
 
 #include <stdbool.h>
+#include <stdint.h>
 
 #define SIM_EFSPN_DC_HOME_PLMN_BIT 0x1
 #define SIM_EFSPN_DC_ROAMING_SPN_BIT 0x2
@@ -299,9 +300,9 @@  enum ber_tlv_data_encoding_type {
 
 struct sim_eons_operator_info {
 	char *longname;
-	gboolean long_ci;
+	bool long_ci;
 	char *shortname;
-	gboolean short_ci;
+	bool short_ci;
 	char *info;
 };
 
@@ -460,18 +461,18 @@  gboolean ber_tlv_builder_recurse_comprehension(struct ber_tlv_builder *builder,
 void ber_tlv_builder_optimize(struct ber_tlv_builder *builder,
 				unsigned char **pdu, unsigned int *len);
 
-struct sim_eons *sim_eons_new(int pnn_records);
-void sim_eons_add_pnn_record(struct sim_eons *eons, int record,
-				const guint8 *tlv, int length);
+struct sim_eons *sim_eons_new(uint32_t pnn_records);
+void sim_eons_add_pnn_record(struct sim_eons *eons, uint32_t record,
+				const uint8_t *tlv, uint16_t length);
 gboolean sim_eons_pnn_is_empty(struct sim_eons *eons);
 void sim_eons_add_opl_record(struct sim_eons *eons,
-				const guint8 *contents, int length);
+				const uint8_t *contents, uint16_t length);
 void sim_eons_optimize(struct sim_eons *eons);
 const struct sim_eons_operator_info *sim_eons_lookup_with_lac(
 						struct sim_eons *eons,
 						const char *mcc,
 						const char *mnc,
-						guint16 lac);
+						uint16_t lac);
 const struct sim_eons_operator_info *sim_eons_lookup(struct sim_eons *eons,
 						const char *mcc,
 						const char *mnc);
diff --git a/unit/test-simutil.c b/unit/test-simutil.c
index 4199991a4d2a..365ef318d937 100644
--- a/unit/test-simutil.c
+++ b/unit/test-simutil.c
@@ -393,7 +393,6 @@  static void test_eons(void)
 	g_assert(!sim_eons_pnn_is_empty(eons_info));
 
 	sim_eons_add_opl_record(eons_info, valid_efopl, sizeof(valid_efopl));
-	sim_eons_optimize(eons_info);
 
 	op_info = sim_eons_lookup(eons_info, "246", "82");
 	g_assert(op_info == NULL);