From e1d32ae355fd50d8d8374cbed846c0f4f8f89b27 Mon Sep 17 00:00:00 2001
From: Suvendhu Hansa <suvendhu.hansa@iopsys.eu>
Date: Thu, 17 Aug 2023 10:12:58 +0000
Subject: [PATCH] bbfdmd: Sort results before pretty_print

---
 bbfdmd/ubus/get_helper.c   | 95 ++++++++++++++++++++++++++++++++++++++
 bbfdmd/ubus/get_helper.h   |  1 +
 bbfdmd/ubus/pretty_print.c | 12 +++++
 3 files changed, 108 insertions(+)

diff --git a/bbfdmd/ubus/get_helper.c b/bbfdmd/ubus/get_helper.c
index 15be1409..9ac288c2 100644
--- a/bbfdmd/ubus/get_helper.c
+++ b/bbfdmd/ubus/get_helper.c
@@ -207,6 +207,72 @@ static int get_random_id(void)
 	return ret;
 }
 
+static int CountConsecutiveDigits(char *p)
+{
+    char c;
+    int num_digits;
+
+    num_digits = 0;
+    c = *p++;
+    while ((c >= '0') && (c <= 9))
+    {
+        num_digits++;
+        c = *p++;
+    }
+
+    return num_digits;
+}
+
+static int compare_path(const void *arg1, const void *arg2)
+{
+	struct pvNode *pv1 = (struct pvNode *)arg1;
+	struct pvNode *pv2 = (struct pvNode *)arg2;
+
+	char *s1 = pv1->param;
+	char *s2 = pv2->param;
+
+	char c1, c2;
+	int num_digits_s1;
+	int num_digits_s2;
+	int delta;
+
+	// Skip all characters which are the same
+	while (true) {
+		c1 = *s1;
+		c2 = *s2;
+
+		// Exit if reached the end of either string
+		if ((c1 == '\0') || (c2 == '\0')) {
+			// NOTE: The following comparision puts s1 before s2, if s1 terminates before s2 (and vice versa)
+			return (int)c1 - (int)c2;
+		}
+
+		// Exit if the characters do not match
+		if (c1 != c2) {
+			break;
+		}
+
+		// As characters match, move to next characters
+		s1++;
+		s2++;
+	}
+
+	// If the code gets here, then we have reached a character which is different
+	// Determine the number of digits in the rest of the string (this may be 0 if the first character is not a digit)
+	num_digits_s1 = CountConsecutiveDigits(s1);
+	num_digits_s2 = CountConsecutiveDigits(s2);
+
+	// Determine if the number of digits in s1 is greater than in s2 (if so, s1 comes after s2)
+	delta = num_digits_s1 - num_digits_s2;
+	if (delta != 0) {
+		return delta;
+	}
+
+	// If the code gets here, then the strings contain either no digits, or the same number of digits,
+	// so just compare the characters (this also works if the characters are digits)
+	return (int)c1 - (int)c2;
+}
+
 // Returns transaction id if successful, otherwise 0
 int transaction_start(uint32_t max_timeout)
 {
@@ -305,3 +371,32 @@ int configure_transaction_timeout(int timeout)
 
 	return 0;
 }
+
+// Returns a pointer to the sorted array of PVs, memory need to be freed by caller
+struct pvNode *sort_pv_path(struct list_head *pv_list, size_t pv_count)
+{
+	if (!pv_list)
+		return NULL;
+
+	if (list_empty(pv_list) || pv_count == 0)
+		return NULL;
+
+	struct pvNode *arr = malloc(sizeof(struct pvNode) * pv_count);
+	if (arr == NULL)
+		return NULL;
+
+	struct pvNode *pv = NULL;
+	size_t i = 0;
+
+	list_for_each_entry(pv, pv_list, list) {
+		if (i == pv_count)
+			break;
+
+		memcpy(&arr[i], pv, sizeof(struct pvNode));
+		i++;
+	}
+
+	qsort(arr, pv_count, sizeof(struct pvNode), compare_path);
+
+	return arr;
+}
diff --git a/bbfdmd/ubus/get_helper.h b/bbfdmd/ubus/get_helper.h
index 40affe1a..e502c2cb 100644
--- a/bbfdmd/ubus/get_helper.h
+++ b/bbfdmd/ubus/get_helper.h
@@ -48,5 +48,6 @@ int transaction_status(struct blob_buf *bb, int trans_id);
 bool is_transaction_running(void);
 bool is_transaction_valid(int trans_id);
 int configure_transaction_timeout(int timeout);
+struct pvNode *sort_pv_path(struct list_head *pv_list, size_t pv_count);
 
 #endif /* GET_HELPER_H */
diff --git a/bbfdmd/ubus/pretty_print.c b/bbfdmd/ubus/pretty_print.c
index bb5c5989..ddcb185d 100644
--- a/bbfdmd/ubus/pretty_print.c
+++ b/bbfdmd/ubus/pretty_print.c
@@ -351,7 +351,17 @@ void prepare_result_blob(struct blob_buf *bb, struct list_head *pv_list)
 	if (list_empty(pv_list))
 		return;
 
+	size_t count = 0;
 	list_for_each_entry(pv, pv_list, list) {
+		count++;
+	}
+
+	struct pvNode *sortedPV = sort_pv_path(pv_list, count);
+	if (sortedPV == NULL)
+		return;
+
+	for (size_t i = 0; i < count; i++) {
+		struct pvNode *pv = &sortedPV[i];
 		ptr = pv->param;
 		if (list_empty(&result_stack)) {
 			DEBUG("stack empty Processing (%s)", ptr);
@@ -382,6 +392,8 @@ void prepare_result_blob(struct blob_buf *bb, struct list_head *pv_list)
 		}
 	}
 
+	free(sortedPV);
+
 	// Close the stack entry if left
 	list_for_each_entry(rnode, &result_stack, list) {
 		blobmsg_close_table(bb, rnode->cookie);