[BlueZ] main: Simplify parse_config_string()

Commit Message

Bastien Nocera May 27, 2024, 8:26 a.m. UTC

The memory management done by parse_config_string() was quite
complicated, as it expected to be able to free the value in the return
variable if it was already allocated.

That particular behaviour was only used for a single variable which was
set to its default value during startup and might be overwritten after
this function call.

Use an intermediate variable to check whether we need to free
btd_opts.name and simplify parse_config_string().

Error: RESOURCE_LEAK (CWE-772): [#def39] [important]
bluez-5.75/src/main.c:425:2: alloc_fn: Storage is returned from allocation function "g_key_file_get_string".
bluez-5.75/src/main.c:425:2: var_assign: Assigning: "tmp" = storage returned from "g_key_file_get_string(config, group, key, &err)".
bluez-5.75/src/main.c:433:2: noescape: Assuming resource "tmp" is not freed or pointed-to as ellipsis argument to "btd_debug".
bluez-5.75/src/main.c:440:2: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to.
438|	}
439|
440|->	return true;
441|   }
442|
---
Essentially a v3 of "main: Simplify variable assignment" with a better
commit message.

 src/main.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

[e-mail] May 27, 2024, 10 a.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=856060

---Test result---

Test Summary:
CheckPatch                    PASS      0.52 seconds
GitLint                       FAIL      0.47 seconds
BuildEll                      PASS      24.03 seconds
BluezMake                     PASS      1611.55 seconds
MakeCheck                     PASS      13.57 seconds
MakeDistcheck                 PASS      172.83 seconds
CheckValgrind                 PASS      245.74 seconds
CheckSmatch                   PASS      346.67 seconds
bluezmakeextell               PASS      117.44 seconds
IncrementalBuild              PASS      1387.87 seconds
ScanBuild                     PASS      978.17 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ] main: Simplify parse_config_string()

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
15: B1 Line exceeds max length (108>80): "bluez-5.75/src/main.c:425:2: alloc_fn: Storage is returned from allocation function "g_key_file_get_string"."
16: B1 Line exceeds max length (132>80): "bluez-5.75/src/main.c:425:2: var_assign: Assigning: "tmp" = storage returned from "g_key_file_get_string(config, group, key, &err)"."
17: B1 Line exceeds max length (126>80): "bluez-5.75/src/main.c:433:2: noescape: Assuming resource "tmp" is not freed or pointed-to as ellipsis argument to "btd_debug"."
18: B1 Line exceeds max length (110>80): "bluez-5.75/src/main.c:440:2: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to."
19: B3 Line contains hard tab characters (\t): "438|	}"
21: B3 Line contains hard tab characters (\t): "440|->	return true;"


---
Regards,
Linux Bluetooth

Patch

diff --git a/src/main.c b/src/main.c
index ac840d684f6d..f6369a20d879 100644
--- a/src/main.c
+++ b/src/main.c
@@ -420,9 +420,10 @@  static bool parse_config_string(GKeyFile *config, const char *group,
 					const char *key, char **val)
 {
 	GError *err = NULL;
-	char *tmp;
 
-	tmp = g_key_file_get_string(config, group, key, &err);
+	g_return_val_if_fail(val, false);
+
+	*val = g_key_file_get_string(config, group, key, &err);
 	if (err) {
 		if (err->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND)
 			DBG("%s", err->message);
@@ -430,12 +431,7 @@  static bool parse_config_string(GKeyFile *config, const char *group,
 		return false;
 	}
 
-	DBG("%s.%s = %s", group, key, tmp);
-
-	if (val) {
-		g_free(*val);
-		*val = tmp;
-	}
+	DBG("%s.%s = %s", group, key, *val);
 
 	return true;
 }
@@ -1005,7 +1001,12 @@  static void parse_secure_conns(GKeyFile *config)
 
 static void parse_general(GKeyFile *config)
 {
-	parse_config_string(config, "General", "Name", &btd_opts.name);
+	char *str = NULL;
+
+	if (parse_config_string(config, "General", "Name", &str)) {
+		g_free(btd_opts.name);
+		btd_opts.name = str;
+	}
 	parse_config_hex(config, "General", "Class", &btd_opts.class);
 	parse_config_u32(config, "General", "DiscoverableTimeout",
 						&btd_opts.discovto,