* [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
* 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
* [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: [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
* [BlueZ v3 1/6] all: Remove more unneeded MIN/MAX macro definition @ 2026-05-11 13:18 Bastien Nocera 2026-05-11 17:27 ` Add helper for "cleanup" variable attribute bluez.test.bot 0 siblings, 1 reply; 10+ messages in thread From: Bastien Nocera @ 2026-05-11 13:18 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
* 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
* [BlueZ 1/5] all: Remove more unneeded MIN/MAX macro definition @ 2026-05-06 9:14 Bastien Nocera 2026-05-06 11:15 ` Add helper for "cleanup" variable attribute bluez.test.bot 0 siblings, 1 reply; 10+ messages in thread From: Bastien Nocera @ 2026-05-06 9:14 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
* 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
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