Linux bluetooth development
 help / color / mirror / Atom feed
* RE: Add helper for "cleanup" variable attribute
  2026-05-06  9:14 [BlueZ 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
@ 2026-05-06 11:15 ` bluez.test.bot
  0 siblings, 0 replies; 10+ messages in thread
From: bluez.test.bot @ 2026-05-06 11:15 UTC (permalink / raw)
  To: linux-bluetooth, hadess

[-- Attachment #1: Type: text/plain, Size: 4626 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    FAIL      1.57 seconds
GitLint                       PASS      1.32 seconds
BuildEll                      PASS      20.73 seconds
BluezMake                     PASS      654.60 seconds
MakeCheck                     PASS      19.06 seconds
MakeDistcheck                 PASS      249.78 seconds
CheckValgrind                 PASS      296.64 seconds
CheckSmatch                   WARNING   355.15 seconds
bluezmakeextell               PASS      184.37 seconds
IncrementalBuild              PASS      1277.75 seconds
ScanBuild                     PASS      1024.24 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,2/5] shared/util: Add helper for "cleanup" variable attribute
ERROR:OPEN_BRACE: open brace '{' following function definitions go on the next line
#74: FILE: src/shared/util.h:96:
+static inline void freep(void *p) {

ERROR:CODE_INDENT: code indent should use tabs where possible
#75: FILE: src/shared/util.h:97:
+        free(*(void**) p);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#75: FILE: src/shared/util.h:97:
+        free(*(void**) p);$

ERROR:POINTER_LOCATION: "(foo**)" should be "(foo **)"
#75: FILE: src/shared/util.h:97:
+        free(*(void**) p);

/github/workspace/src/patch/14557019.patch total: 3 errors, 1 warnings, 13 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/github/workspace/src/patch/14557019.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,5/5] main: Use _cleanup_() to simplify GError-handling
ERROR:OPEN_BRACE: open brace '{' following function definitions go on the next line
#66: FILE: src/main.c:208:
+static inline void free_error(void *err) {

ERROR:CODE_INDENT: code indent should use tabs where possible
#67: FILE: src/main.c:209:
+        g_error_free(*(void**) err);$

WARNING:LEADING_SPACE: please, no spaces at the start of a line
#67: FILE: src/main.c:209:
+        g_error_free(*(void**) err);$

ERROR:POINTER_LOCATION: "(foo**)" should be "(foo **)"
#67: FILE: src/main.c:209:
+        g_error_free(*(void**) err);

/github/workspace/src/patch/14557018.patch total: 3 errors, 1 warnings, 127 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/github/workspace/src/patch/14557018.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/main.c: note: in included file (through src/device.h):src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/main.c: note: in included file (through src/device.h):src/main.c: note: in included file (through src/device.h):


https://github.com/bluez/bluez/pull/2101

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [BlueZ v2 0/5] Add helper for "cleanup" variable attribute
@ 2026-05-06 14:30 Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Bastien Nocera @ 2026-05-06 14:30 UTC (permalink / raw)
  To: linux-bluetooth

As discussed in:
https://lore.kernel.org/linux-bluetooth/ed949f2550f79a4bef19bd482bf8b069ad5b7e0c.camel@hadess.net/

Implement a cleanup helper.

The MIN/MAX fix is here because it touches the same hunk in src/main.c
as the other patches. Feel free to pick it up straight away while the
rest is discussed.

Changes since v1:
- Fixed checkpatch warnings

Bastien Nocera (5):
  all: Remove more unneeded MIN/MAX macro definition
  shared/util: Add helper for "cleanup" variable attribute
  doc: Recommend using _cleanup_ and friends
  main: Use _cleanup_() to simplify configuration parsing
  main: Use _cleanup_() to simplify GError-handling

 doc/maintainer-guidelines.rst |   3 +
 lib/bluetooth/hci.c           |   4 --
 src/main.c                    | 102 +++++++++++-----------------------
 src/shared/gatt-server.c      |   8 ---
 src/shared/util.h             |   8 +++
 5 files changed, 43 insertions(+), 82 deletions(-)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [BlueZ v2 1/5] all: Remove more unneeded MIN/MAX macro definition
  2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
@ 2026-05-06 14:30 ` Bastien Nocera
  2026-05-06 21:28   ` Add helper for "cleanup" variable attribute bluez.test.bot
  2026-05-06 14:30 ` [BlueZ v2 2/5] shared/util: " Bastien Nocera
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Bastien Nocera @ 2026-05-06 14:30 UTC (permalink / raw)
  To: linux-bluetooth

---
 lib/bluetooth/hci.c      | 4 ----
 src/main.c               | 4 ----
 src/shared/gatt-server.c | 8 --------
 3 files changed, 16 deletions(-)

diff --git a/lib/bluetooth/hci.c b/lib/bluetooth/hci.c
index 44eea054b0ac..f50a30c610be 100644
--- a/lib/bluetooth/hci.c
+++ b/lib/bluetooth/hci.c
@@ -33,10 +33,6 @@
 #include "hci.h"
 #include "hci_lib.h"
 
-#ifndef MIN
-#define MIN(x, y) ((x) < (y) ? (x) : (y))
-#endif
-
 typedef struct {
 	char *str;
 	unsigned int val;
diff --git a/src/main.c b/src/main.c
index 9a3d2da25d4d..8aa19a3e3346 100644
--- a/src/main.c
+++ b/src/main.c
@@ -205,10 +205,6 @@ static const struct group_table {
 	{ }
 };
 
-#ifndef MIN
-#define MIN(x, y) ((x) < (y) ? (x) : (y))
-#endif
-
 static int8_t check_sirk_alpha_numeric(char *str)
 {
 	int8_t val = 0;
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 6273899965c0..709a8f94bb6a 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -26,14 +26,6 @@
 #include "src/shared/util.h"
 #include "src/shared/timeout.h"
 
-#ifndef MAX
-#define MAX(a, b) ((a) > (b) ? (a) : (b))
-#endif
-
-#ifndef MIN
-#define MIN(a, b) ((a) < (b) ? (a) : (b))
-#endif
-
 /*
  * TODO: This is an arbitrary limit. Come up with something reasonable or
  * perhaps an API to set this value if there is a use case for it.
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [BlueZ v2 2/5] shared/util: Add helper for "cleanup" variable attribute
  2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
@ 2026-05-06 14:30 ` Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 3/5] doc: Recommend using _cleanup_ and friends Bastien Nocera
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2026-05-06 14:30 UTC (permalink / raw)
  To: linux-bluetooth

Use the widespread "cleanup" variable attribute:
https://gcc.gnu.org/onlinedocs/gcc/Common-Attributes.html#index-cleanup

It is implemented by both GCC and clang on platforms where bluez is
used, and can help reduce memory leaks, while improving readability.

See also:
https://systemd.io/CODING_STYLE/#memory-allocation
https://docs.gtk.org/glib/auto-cleanup.html
---
 src/shared/util.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/shared/util.h b/src/shared/util.h
index 67629dddfaa9..1660b6ea5149 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -93,6 +93,14 @@ do {						\
 #define newa(t, n) ((t*) alloca(sizeof(t)*(n)))
 #define malloc0(n) (calloc(1, (n)))
 
+static inline void freep(void *p)
+{
+	free(*(void **) p);
+}
+
+#define _cleanup_(f) __attribute__((cleanup(f)))
+#define _cleanup_free_ _cleanup_(freep)
+
 char *strdelimit(char *str, char *del, char c);
 int strsuffix(const char *str, const char *suffix);
 char *strstrip(char *str);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [BlueZ v2 3/5] doc: Recommend using _cleanup_ and friends
  2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 2/5] shared/util: " Bastien Nocera
@ 2026-05-06 14:30 ` Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 4/5] main: Use _cleanup_() to simplify configuration parsing Bastien Nocera
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2026-05-06 14:30 UTC (permalink / raw)
  To: linux-bluetooth

---
 doc/maintainer-guidelines.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/maintainer-guidelines.rst b/doc/maintainer-guidelines.rst
index 44d3e258db6e..b67c6596f4c1 100644
--- a/doc/maintainer-guidelines.rst
+++ b/doc/maintainer-guidelines.rst
@@ -98,6 +98,9 @@ do this:
 
 The above assumes that a kernel tree resides in ``~/src/linux/``.
 
+Also make sure to use ``_cleanup_free_`` and ``_cleanup_(free_func)`` when
+possible. It makes your code much nicer to read (and shorter), and avoids
+common memory leaks on error paths.
 
 Rule 4: Pay extra attention to adding new files to the tree
 -----------------------------------------------------------
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [BlueZ v2 4/5] main: Use _cleanup_() to simplify configuration parsing
  2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
                   ` (2 preceding siblings ...)
  2026-05-06 14:30 ` [BlueZ v2 3/5] doc: Recommend using _cleanup_ and friends Bastien Nocera
@ 2026-05-06 14:30 ` Bastien Nocera
  2026-05-06 14:30 ` [BlueZ v2 5/5] main: Use _cleanup_() to simplify GError-handling Bastien Nocera
  2026-05-07 14:42 ` [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
  5 siblings, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2026-05-06 14:30 UTC (permalink / raw)
  To: linux-bluetooth

Makes dealing with the error path easier.
---
 src/main.c | 70 ++++++++++++++++--------------------------------------
 1 file changed, 20 insertions(+), 50 deletions(-)

diff --git a/src/main.c b/src/main.c
index 8aa19a3e3346..3a2c3e645a74 100644
--- a/src/main.c
+++ b/src/main.c
@@ -462,7 +462,7 @@ static bool parse_config_int(GKeyFile *config, const char *group,
 					size_t min, size_t max)
 {
 	size_t tmp;
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 	char *endptr = NULL;
 
 	if (!parse_config_string(config, group, key, &str))
@@ -471,25 +471,21 @@ static bool parse_config_int(GKeyFile *config, const char *group,
 	tmp = strtol(str, &endptr, 0);
 	if (!endptr || *endptr != '\0') {
 		error("%s.%s = %s is not integer", group, key, str);
-		g_free(str);
 		return false;
 	}
 
 	if (tmp < min) {
-		g_free(str);
 		warn("%s.%s = %zu is out of range (< %zu)", group, key, tmp,
 									min);
 		return false;
 	}
 
 	if (tmp > max) {
-		g_free(str);
 		warn("%s.%s = %zu is out of range (> %zu)", group, key, tmp,
 									max);
 		return false;
 	}
 
-	g_free(str);
 	if (val)
 		*val = tmp;
 
@@ -500,10 +496,9 @@ static bool parse_config_signed_int(GKeyFile *config, const char *group,
 					const char *key, int8_t *val,
 					long min, long max)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 	char *endptr = NULL;
 	long tmp;
-	bool result = false;
 
 	str = g_key_file_get_string(config, group, key, NULL);
 	if (!str)
@@ -512,28 +507,24 @@ static bool parse_config_signed_int(GKeyFile *config, const char *group,
 	tmp = strtol(str, &endptr, 0);
 	if (!endptr || *endptr != '\0') {
 		warn("%s.%s = %s is not integer", group, key, str);
-		goto cleanup;
+		return false;
 	}
 
 	if (tmp < min) {
 		warn("%s.%s = %ld is out of range (< %ld)", group, key, tmp,
 			min);
-		goto cleanup;
+		return false;
 	}
 
 	if (tmp > max) {
 		warn("%s.%s = %ld is out of range (> %ld)", group, key, tmp,
 									max);
-		goto cleanup;
+		return false;
 	}
 
 	if (val)
 		*val = (int8_t) tmp;
-	result = true;
-
-cleanup:
-	g_free(str);
-	return result;
+	return true;
 }
 
 struct config_param {
@@ -915,7 +906,7 @@ static bool parse_config_bool(GKeyFile *config, const char *group,
 
 static void parse_privacy(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "General", "Privacy", &str)) {
 		btd_opts.privacy = 0x00;
@@ -948,13 +939,11 @@ static void parse_privacy(GKeyFile *config)
 		DBG("Invalid privacy option: %s", str);
 		btd_opts.privacy = 0x00;
 	}
-
-	g_free(str);
 }
 
 static void parse_repairing(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "General", "JustWorksRepairing",
 						&str)) {
@@ -963,13 +952,12 @@ static void parse_repairing(GKeyFile *config)
 	}
 
 	btd_opts.jw_repairing = parse_jw_repairing(str);
-	g_free(str);
 }
 
 static bool parse_config_hex(GKeyFile *config, char *group,
 					const char *key, uint32_t *val)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, group, key, &str))
 		return false;
@@ -977,37 +965,34 @@ static bool parse_config_hex(GKeyFile *config, char *group,
 	if (val)
 		*val = strtol(str, NULL, 16);
 
-	g_free(str);
 	return true;
 }
 
 static void parse_device_id(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	parse_config_string(config, "General", "DeviceID", &str);
 	if (!str)
 		return;
 
 	parse_did(str);
-	g_free(str);
 }
 
 static void parse_ctrl_mode(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	parse_config_string(config, "General", "ControllerMode", &str);
 	if (!str)
 		return;
 
 	btd_opts.mode = get_mode(str);
-	g_free(str);
 }
 
 static void parse_multi_profile(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	parse_config_string(config, "General", "MultiProfile", &str);
 	if (!str)
@@ -1019,8 +1004,6 @@ static void parse_multi_profile(GKeyFile *config)
 		btd_opts.mps = MPS_MULTIPLE;
 	else
 		btd_opts.mps = MPS_OFF;
-
-	g_free(str);
 }
 
 static gboolean parse_kernel_experimental(const char *key, const char *value,
@@ -1043,20 +1026,18 @@ static gboolean parse_kernel_experimental(const char *key, const char *value,
 
 static void parse_kernel_exp(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "General", "KernelExperimental",
 						&str))
 		return;
 
 	parse_kernel_experimental(NULL, str, NULL, NULL);
-
-	g_free(str);
 }
 
 static void parse_secure_conns(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "General", "SecureConnections",
 								&str))
@@ -1068,8 +1049,6 @@ static void parse_secure_conns(GKeyFile *config)
 		btd_opts.secure_conn = SC_ON;
 	else if (!strcmp(str, "only"))
 		btd_opts.secure_conn = SC_ONLY;
-
-	g_free(str);
 }
 
 static void parse_general(GKeyFile *config)
@@ -1120,14 +1099,13 @@ static void parse_general(GKeyFile *config)
 
 static void parse_gatt_cache(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	parse_config_string(config, "GATT", "Cache", &str);
 	if (!str)
 		return;
 
 	btd_opts.gatt_cache = parse_gatt_cache_str(str);
-	g_free(str);
 }
 
 static enum bt_gatt_export_t parse_gatt_export_str(const char *str)
@@ -1147,14 +1125,13 @@ static enum bt_gatt_export_t parse_gatt_export_str(const char *str)
 
 static void parse_gatt_export(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	parse_config_string(config, "GATT", "ExportClaimedServices", &str);
 	if (!str)
 		return;
 
 	btd_opts.gatt_export = parse_gatt_export_str(str);
-	g_free(str);
 }
 
 static uint8_t parse_gatt_seclevel_str(const char *str)
@@ -1176,7 +1153,7 @@ static uint8_t parse_gatt_seclevel_str(const char *str)
 
 static void parse_gatt_seclevel(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!btd_opts.testing)
 		return;
@@ -1186,7 +1163,6 @@ static void parse_gatt_seclevel(GKeyFile *config)
 		return;
 
 	btd_opts.gatt_seclevel = parse_gatt_seclevel_str(str);
-	g_free(str);
 }
 
 static void parse_gatt(GKeyFile *config)
@@ -1204,7 +1180,7 @@ static void parse_gatt(GKeyFile *config)
 
 static void parse_csis_sirk(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "CSIS", "SIRK", &str))
 		return;
@@ -1213,8 +1189,6 @@ static void parse_csis_sirk(GKeyFile *config)
 		hex2bin(str, btd_opts.csis.sirk, sizeof(btd_opts.csis.sirk));
 	else if (!gen_sirk(str))
 		DBG("Unable to generate SIRK from string");
-
-	g_free(str);
 }
 
 static void parse_csis(GKeyFile *config)
@@ -1305,7 +1279,7 @@ static void parse_le_cs_config(GKeyFile *config)
 
 static void parse_avdtp_session_mode(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "AVDTP", "SessionMode", &str))
 		return;
@@ -1318,13 +1292,11 @@ static void parse_avdtp_session_mode(GKeyFile *config)
 		DBG("Invalid mode option: %s", str);
 		btd_opts.avdtp.session_mode = BT_IO_MODE_BASIC;
 	}
-
-	g_free(str);
 }
 
 static void parse_avdtp_stream_mode(GKeyFile *config)
 {
-	char *str = NULL;
+	_cleanup_(g_free) char *str = NULL;
 
 	if (!parse_config_string(config, "AVDTP", "StreamMode", &str))
 		return;
@@ -1337,8 +1309,6 @@ static void parse_avdtp_stream_mode(GKeyFile *config)
 		DBG("Invalid mode option: %s", str);
 		btd_opts.avdtp.stream_mode = BT_IO_MODE_BASIC;
 	}
-
-	g_free(str);
 }
 
 static void parse_avdtp(GKeyFile *config)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [BlueZ v2 5/5] main: Use _cleanup_() to simplify GError-handling
  2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
                   ` (3 preceding siblings ...)
  2026-05-06 14:30 ` [BlueZ v2 4/5] main: Use _cleanup_() to simplify configuration parsing Bastien Nocera
@ 2026-05-06 14:30 ` Bastien Nocera
  2026-05-07 14:42 ` [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
  5 siblings, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2026-05-06 14:30 UTC (permalink / raw)
  To: linux-bluetooth

Use _cleanup_() to simplify GError-handling in the error paths.
---
 src/main.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/main.c b/src/main.c
index 3a2c3e645a74..ef11b07d7a9a 100644
--- a/src/main.c
+++ b/src/main.c
@@ -205,6 +205,13 @@ static const struct group_table {
 	{ }
 };
 
+static inline void free_error(void *err)
+{
+	g_error_free(*(void **) err);
+}
+
+#define _cleanup_error_ _cleanup_(free_error)
+
 static int8_t check_sirk_alpha_numeric(char *str)
 {
 	int8_t val = 0;
@@ -252,7 +259,7 @@ GKeyFile *btd_get_main_conf(void)
 
 static GKeyFile *load_config(const char *name)
 {
-	GError *err = NULL;
+	_cleanup_error_ GError *err = NULL;
 	GKeyFile *keyfile;
 	int len;
 
@@ -285,7 +292,6 @@ static GKeyFile *load_config(const char *name)
 		if (!g_error_matches(err, G_FILE_ERROR, G_FILE_ERROR_NOENT))
 			error("Parsing %s failed: %s", main_conf_file_path,
 				err->message);
-		g_error_free(err);
 		g_key_file_free(keyfile);
 		return NULL;
 	}
@@ -436,14 +442,13 @@ static int get_mode(const char *str)
 static bool parse_config_string(GKeyFile *config, const char *group,
 					const char *key, char **val)
 {
-	GError *err = NULL;
+	_cleanup_error_ GError *err = NULL;
 	char *tmp;
 
 	tmp = 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);
-		g_error_free(err);
 		return false;
 	}
 
@@ -885,14 +890,13 @@ static bool parse_config_u8(GKeyFile *config, const char *group,
 static bool parse_config_bool(GKeyFile *config, const char *group,
 					const char *key, bool *val)
 {
-	GError *err = NULL;
+	_cleanup_error_ GError *err = NULL;
 	gboolean tmp;
 
 	tmp = g_key_file_get_boolean(config, group, key, &err);
 	if (err) {
 		if (err->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND)
 			DBG("%s", err->message);
-		g_error_free(err);
 		return false;
 	}
 
@@ -1205,8 +1209,8 @@ static void parse_csis(GKeyFile *config)
 static bool parse_cs_role(GKeyFile *config, const char *group,
 					const char *key, uint8_t *val)
 {
-	GError *err = NULL;
-	char *str = NULL;
+	_cleanup_error_ GError *err = NULL;
+	_cleanup_(g_free) char *str = NULL;
 	char *endptr = NULL;
 	int numeric_val;
 
@@ -1215,7 +1219,6 @@ static bool parse_cs_role(GKeyFile *config, const char *group,
 	if (err) {
 		if (err->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND)
 			DBG("%s", err->message);
-		g_error_free(err);
 		return false;
 	}
 
@@ -1225,17 +1228,14 @@ static bool parse_cs_role(GKeyFile *config, const char *group,
 	if (!strcmp(str, "Initiator") || !strcmp(str, "initiator")) {
 		if (val)
 			*val = 1;
-		g_free(str);
 		return true;
 	} else if (!strcmp(str, "Reflector") || !strcmp(str, "reflector")) {
 		if (val)
 			*val = 2;
-		g_free(str);
 		return true;
 	} else if (!strcmp(str, "Both") || !strcmp(str, "both")) {
 		if (val)
 			*val = 3;
-		g_free(str);
 		return true;
 	}
 
@@ -1246,7 +1246,6 @@ static bool parse_cs_role(GKeyFile *config, const char *group,
 		warn("%s.%s = %s is not a valid value. "
 			"Expected: 1/Initiator, 2/Reflector, or 3/Both",
 			group, key, str);
-		g_free(str);
 		return false;
 	}
 
@@ -1254,14 +1253,12 @@ static bool parse_cs_role(GKeyFile *config, const char *group,
 		warn("%s.%s = %d is out of range. "
 			"Valid values: 1 (Initiator), 2 (Reflector), 3 (Both)",
 			group, key, numeric_val);
-		g_free(str);
 		return false;
 	}
 
 	if (val)
 		*val = numeric_val;
 
-	g_free(str);
 	return true;
 }
 
@@ -1575,7 +1572,7 @@ static GOptionEntry options[] = {
 int main(int argc, char *argv[])
 {
 	GOptionContext *context;
-	GError *err = NULL;
+	_cleanup_error_ GError *err = NULL;
 	uint16_t sdp_mtu = 0;
 	uint32_t sdp_flags = 0;
 	int gdbus_flags = 0;
@@ -1588,7 +1585,6 @@ int main(int argc, char *argv[])
 	if (g_option_context_parse(context, &argc, &argv, &err) == FALSE) {
 		if (err != NULL) {
 			g_printerr("%s\n", err->message);
-			g_error_free(err);
 		} else
 			g_printerr("An unknown error occurred\n");
 		exit(1);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: Add helper for "cleanup" variable attribute
  2026-05-06 14:30 ` [BlueZ v2 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
@ 2026-05-06 21:28   ` bluez.test.bot
  0 siblings, 0 replies; 10+ messages in thread
From: bluez.test.bot @ 2026-05-06 21:28 UTC (permalink / raw)
  To: linux-bluetooth, hadess

[-- Attachment #1: Type: text/plain, Size: 1922 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PASS      2.00 seconds
GitLint                       PASS      1.32 seconds
BuildEll                      PASS      21.42 seconds
BluezMake                     PASS      638.21 seconds
MakeCheck                     PASS      18.99 seconds
MakeDistcheck                 PASS      232.55 seconds
CheckValgrind                 PASS      272.18 seconds
CheckSmatch                   WARNING   331.14 seconds
bluezmakeextell               PASS      163.78 seconds
IncrementalBuild              PASS      1203.38 seconds
ScanBuild                     PASS      924.01 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/main.c: note: in included file (through src/device.h):src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/main.c: note: in included file (through src/device.h):src/main.c: note: in included file (through src/device.h):


https://github.com/bluez/bluez/pull/2102

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [BlueZ v2 0/5] Add helper for "cleanup" variable attribute
  2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
                   ` (4 preceding siblings ...)
  2026-05-06 14:30 ` [BlueZ v2 5/5] main: Use _cleanup_() to simplify GError-handling Bastien Nocera
@ 2026-05-07 14:42 ` Bastien Nocera
  5 siblings, 0 replies; 10+ messages in thread
From: Bastien Nocera @ 2026-05-07 14:42 UTC (permalink / raw)
  To: linux-bluetooth

Hey,

I've made some changes to this patchset locally to make it easier to
use, I'll send a v3 next week.

(the MIN/MAX changes are still fine to push right now)

Regards

On Wed, 2026-05-06 at 16:30 +0200, Bastien Nocera wrote:
> As discussed in:
> https://lore.kernel.org/linux-bluetooth/ed949f2550f79a4bef19bd482bf8b069ad5b7e0c.camel@hadess.net/
> 
> Implement a cleanup helper.
> 
> The MIN/MAX fix is here because it touches the same hunk in
> src/main.c
> as the other patches. Feel free to pick it up straight away while the
> rest is discussed.
> 
> Changes since v1:
> - Fixed checkpatch warnings
> 
> Bastien Nocera (5):
>   all: Remove more unneeded MIN/MAX macro definition
>   shared/util: Add helper for "cleanup" variable attribute
>   doc: Recommend using _cleanup_ and friends
>   main: Use _cleanup_() to simplify configuration parsing
>   main: Use _cleanup_() to simplify GError-handling
> 
>  doc/maintainer-guidelines.rst |   3 +
>  lib/bluetooth/hci.c           |   4 --
>  src/main.c                    | 102 +++++++++++---------------------
> --
>  src/shared/gatt-server.c      |   8 ---
>  src/shared/util.h             |   8 +++
>  5 files changed, 43 insertions(+), 82 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: Add helper for "cleanup" variable attribute
  2026-05-11 13:18 [BlueZ v3 1/6] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
@ 2026-05-11 17:27 ` bluez.test.bot
  0 siblings, 0 replies; 10+ messages in thread
From: bluez.test.bot @ 2026-05-11 17:27 UTC (permalink / raw)
  To: linux-bluetooth, hadess

[-- Attachment #1: Type: text/plain, Size: 5159 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    FAIL      2.71 seconds
GitLint                       PASS      1.76 seconds
BuildEll                      PASS      20.40 seconds
BluezMake                     PASS      666.47 seconds
MakeCheck                     PASS      18.72 seconds
MakeDistcheck                 PASS      245.36 seconds
CheckValgrind                 PASS      291.26 seconds
CheckSmatch                   WARNING   351.83 seconds
bluezmakeextell               PASS      186.73 seconds
IncrementalBuild              PASS      1330.28 seconds
ScanBuild                     PASS      1038.55 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v3,2/6] shared/util: Add helper for "cleanup" variable attribute
ERROR:POINTER_LOCATION: "foo * bar" should be "foo *bar"
#106: FILE: src/shared/util.h:103:
+static inline void * _steal_(void *p)

WARNING:LONG_LINE: line length of 104 exceeds 80 columns
#132: FILE: src/shared/util.h:129:
+#define CLEANUP_FREEFUNC(type, func) static inline void cleanup_##type (type **_ptr) { (func) (*_ptr); }

WARNING:SPACING: space prohibited between function name and open parenthesis '('
#132: FILE: src/shared/util.h:129:
+#define CLEANUP_FREEFUNC(type, func) static inline void cleanup_##type (type **_ptr) { (func) (*_ptr); }

ERROR:SPACING: need consistent spacing around '*' (ctx:WxO)
#132: FILE: src/shared/util.h:129:
+#define CLEANUP_FREEFUNC(type, func) static inline void cleanup_##type (type **_ptr) { (func) (*_ptr); }
                                                                              ^

WARNING:LINE_SPACING: Missing a blank line after declarations
#184: FILE: unit/test-util.c:46:
+		_cleanup_free_ uint8_t *data = NULL;
+		data = malloc0(128 * 2);

/github/workspace/src/patch/14565706.patch total: 2 errors, 3 warnings, 140 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14565706.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


[BlueZ,v3,4/6] main: Use _cleanup_() to simplify configuration parsing
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#84: FILE: src/main.c:258:
+	_cleanup_type_(GError) GError *err = NULL;
 	                              ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#108: FILE: src/main.c:440:
+	_cleanup_type_(GError) GError *err = NULL;
 	                              ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#204: FILE: src/main.c:888:
+	_cleanup_type_(GError) GError *err = NULL;
 	                              ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#409: FILE: src/main.c:1207:
+	_cleanup_type_(GError) GError *err = NULL;
 	                              ^

ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#501: FILE: src/main.c:1570:
+	_cleanup_type_(GError) GError *err = NULL;
 	                              ^

/github/workspace/src/patch/14565665.patch total: 5 errors, 0 warnings, 413 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14565665.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/main.c: note: in included file (through src/device.h):src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/shared/gatt-server.c:271:25: warning: Variable length array is used.src/shared/gatt-server.c:614:25: warning: Variable length array is used.src/shared/gatt-server.c:712:25: warning: Variable length array is used.src/main.c: note: in included file (through src/device.h):


https://github.com/bluez/bluez/pull/2118

---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-11 17:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 14:30 [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
2026-05-06 14:30 ` [BlueZ v2 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
2026-05-06 21:28   ` Add helper for "cleanup" variable attribute bluez.test.bot
2026-05-06 14:30 ` [BlueZ v2 2/5] shared/util: " Bastien Nocera
2026-05-06 14:30 ` [BlueZ v2 3/5] doc: Recommend using _cleanup_ and friends Bastien Nocera
2026-05-06 14:30 ` [BlueZ v2 4/5] main: Use _cleanup_() to simplify configuration parsing Bastien Nocera
2026-05-06 14:30 ` [BlueZ v2 5/5] main: Use _cleanup_() to simplify GError-handling Bastien Nocera
2026-05-07 14:42 ` [BlueZ v2 0/5] Add helper for "cleanup" variable attribute Bastien Nocera
  -- strict thread matches above, loose matches on Subject: below --
2026-05-11 13:18 [BlueZ v3 1/6] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
2026-05-11 17:27 ` Add helper for "cleanup" variable attribute bluez.test.bot
2026-05-06  9:14 [BlueZ 1/5] all: Remove more unneeded MIN/MAX macro definition Bastien Nocera
2026-05-06 11:15 ` Add helper for "cleanup" variable attribute bluez.test.bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox