[BlueZ 0/6] "cleanup" variable attribute follow-up

[BlueZ 0/6] "cleanup" variable attribute follow-up

[Bastien Nocera]

Follow-up to the partially applied "Add helper for "cleanup" variable
attribute" patchset.

This new version:
- fixes the crasher seen starting up bluetoothd
- fixes the warnings seen starting up bluetoothd, seen when free'ing
  NULL variables
- reimplements the _steal_ functions as macros to better type-check at
  compile-time
- mentions _steal_ macros in the style docs.

The original patchset also caught a bug in the meson port where the
config file path wasn't made absolute, so bluetoothd couldn't find the
config file when starting up, so I never saw those warnings and crashes.
This bug is now fixed in the meson port branch.

Bastien Nocera (6):
  shared/util: Fix warnings when cleaning up NULL pointers
  shared/util: Better type-checking in _steal_* commands
  doc: Recommend using _cleanup_ and friends
  main: Use _cleanup_() to simplify configuration parsing
  client: Use _cleanup_fd_ to simplify urandom access
  btattach: Use _cleanup_fd_ to simplify error paths

 client/mgmt.c                 |   5 +-
 doc/maintainer-guidelines.rst |   6 ++
 src/main.c                    | 108 ++++++++++++----------------------
 src/shared/util.h             |  23 +++-----
 tools/btattach.c              |  18 ++----
 5 files changed, 60 insertions(+), 100 deletions(-)

-- 
2.54.0

[BlueZ 1/6] shared/util: Fix warnings when cleaning up NULL pointers

[Bastien Nocera]

Don't try to free NULL pointers in cleanup callbacks, some of the
functions we use to free pointers might not support being passed
non-NULL values.

Fixes: 2e0533f977cc ("shared/util: Add helper for "cleanup" variable attribute")
---
 src/shared/util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/util.h b/src/shared/util.h
index 562a5af31751..dd8e5288f55b 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -127,7 +127,7 @@ static inline void closefd(int *fdp)
 }
 
 #define CLEANUP_FREEFUNC(type, func) \
-	static inline void cleanup_##type (type **_ptr) { (func) (*_ptr); }
+	static inline void cleanup_##type (type **_ptr) { if (*_ptr != NULL) (func) (*_ptr); }
 
 #define _cleanup_(f) __attribute__((cleanup(f)))
 #define _cleanup_type_(type) __attribute__((cleanup(cleanup_##type)))
-- 
2.54.0

RE: "cleanup" variable attribute follow-up

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 4250 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=1108616

---Test result---

Test Summary:
CheckPatch                    FAIL      2.51 seconds
GitLint                       PASS      1.73 seconds
BuildEll                      PASS      16.19 seconds
BluezMake                     PASS      498.81 seconds
MakeCheck                     PASS      18.30 seconds
MakeDistcheck                 PASS      189.35 seconds
CheckValgrind                 PASS      217.95 seconds
CheckSmatch                   WARNING   253.71 seconds
bluezmakeextell               PASS      131.42 seconds
IncrementalBuild              PASS      985.31 seconds
ScanBuild                     PASS      733.50 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/6] shared/util: Fix warnings when cleaning up NULL pointers
WARNING:LONG_LINE: line length of 94 exceeds 80 columns
#74: FILE: src/shared/util.h:130:
+	static inline void cleanup_##type (type **_ptr) { if (*_ptr != NULL) (func) (*_ptr); }

WARNING:SPACING: space prohibited between function name and open parenthesis '('
#74: FILE: src/shared/util.h:130:
+	static inline void cleanup_##type (type **_ptr) { if (*_ptr != NULL) (func) (*_ptr); }

ERROR:SPACING: need consistent spacing around '*' (ctx:WxO)
#74: FILE: src/shared/util.h:130:
+	static inline void cleanup_##type (type **_ptr) { if (*_ptr != NULL) (func) (*_ptr); }
 	                                        ^

ERROR:TRAILING_STATEMENTS: trailing statements should be on next line
#74: FILE: src/shared/util.h:130:
+	static inline void cleanup_##type (type **_ptr) { if (*_ptr != NULL) (func) (*_ptr); }

/github/workspace/src/patch/14619750.patch total: 2 errors, 2 warnings, 8 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/14619750.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,4/6] main: Use _cleanup_() to simplify configuration parsing
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#90: FILE: src/main.c:265:
+	_cleanup_type_(GError) GError *err = NULL;
 	                              ^

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

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

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

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

/github/workspace/src/patch/14619699.patch total: 5 errors, 0 warnings, 420 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/14619699.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):


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

---
Regards,
Linux Bluetooth

[BlueZ 2/6] shared/util: Better type-checking in _steal_* commands

[Bastien Nocera]

Replace _steal_() and _steal_fd_() inline functions with macros, so that
we can have better type-checking. This would have avoided crashes seen
in the original patchset, where we passed a pointer instead of a pointer
to a pointer.

Example usage:
CLEANUP_FREEFUNC(GKeyFile, g_key_file_free);
GKeyfile *load_config(void) {
  _cleanup_type_(GKeyFile) GKeyFile *keyfile = NULL;
  [...]
  return _steal_(keyfile);
}

static int open_file(void) {
  _cleanup_fd_ int fd = -1;
  [...]
  return _steal_fd_(fd);
}

This is inspired by similar code in Pipewire's spa helpers, as pointed
out by Pauli Virtanen <pav@iki.fi>.
---
 src/shared/util.h | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/shared/util.h b/src/shared/util.h
index dd8e5288f55b..dacbff67f067 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -100,20 +100,15 @@ static inline void freep(void *p)
 	free(*(void **) p);
 }
 
-static inline void *_steal_(void *p)
-{
-	void **orig = (void **) p;
-	void *ret = *orig;
-	*orig = NULL;
-	return ret;
-}
+#define _exchange_(var, new_value) ({		\
+	__typeof__(var) *orig = &(var);		\
+	__typeof__(var) ret = *orig;		\
+	*orig = new_value;			\
+	ret;					\
+})
 
-static inline int _steal_fd_(int *fdp)
-{
-	int fd = *fdp;
-	*fdp = -1;
-	return fd;
-}
+#define _steal_(p) _exchange_(p, NULL)
+#define _steal_fd_(fd) _exchange_(fd, -1)
 
 static inline void closefd(int *fdp)
 {
-- 
2.54.0

[BlueZ 3/6] doc: Recommend using _cleanup_ and friends

[Bastien Nocera]

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

diff --git a/doc/maintainer-guidelines.rst b/doc/maintainer-guidelines.rst
index 44d3e258db6e..6f2d63d7c76a 100644
--- a/doc/maintainer-guidelines.rst
+++ b/doc/maintainer-guidelines.rst
@@ -98,6 +98,12 @@ 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.
+
+If you need to return a variable that would normally be cleaned up by
+``_cleanup_free_``, use ``_steal_``.
 
 Rule 4: Pay extra attention to adding new files to the tree
 -----------------------------------------------------------
-- 
2.54.0

[BlueZ 4/6] main: Use _cleanup_() to simplify configuration parsing

[Bastien Nocera]

Use helpers to simplify temporary string usage, and cleanup in error
paths.
---
 src/main.c | 108 +++++++++++++++++++----------------------------------
 1 file changed, 38 insertions(+), 70 deletions(-)

diff --git a/src/main.c b/src/main.c
index 8aa19a3e3346..bde295986869 100644
--- a/src/main.c
+++ b/src/main.c
@@ -205,6 +205,16 @@ static const struct group_table {
 	{ }
 };
 
+CLEANUP_FREEFUNC(GError, g_error_free);
+CLEANUP_FREEFUNC(GKeyFile, g_key_file_free);
+
+static inline void cleanup_g_free(void *p)
+{
+	g_free(*(void **) p);
+}
+
+#define _cleanup_g_free_ _cleanup_(cleanup_g_free)
+
 static int8_t check_sirk_alpha_numeric(char *str)
 {
 	int8_t val = 0;
@@ -252,8 +262,8 @@ GKeyFile *btd_get_main_conf(void)
 
 static GKeyFile *load_config(const char *name)
 {
-	GError *err = NULL;
-	GKeyFile *keyfile;
+	_cleanup_type_(GError) GError *err = NULL;
+	_cleanup_type_(GKeyFile) GKeyFile *keyfile = NULL;
 	int len;
 
 	if (name)
@@ -285,12 +295,10 @@ 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;
 	}
 
-	return keyfile;
+	return _steal_(keyfile);
 }
 
 static void parse_did(const char *did)
@@ -436,14 +444,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_type_(GError) 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;
 	}
 
@@ -462,7 +469,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 +478,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 +503,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 +514,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 {
@@ -894,14 +892,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_type_(GError) 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;
 	}
 
@@ -915,7 +912,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 +945,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 +958,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 +971,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 +1010,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 +1032,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 +1055,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 +1105,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 +1131,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 +1159,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 +1169,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 +1186,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 +1195,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)
@@ -1231,8 +1211,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_type_(GError) GError *err = NULL;
+	_cleanup_g_free_ char *str = NULL;
 	char *endptr = NULL;
 	int numeric_val;
 
@@ -1241,7 +1221,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;
 	}
 
@@ -1251,17 +1230,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;
 	}
 
@@ -1272,7 +1248,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;
 	}
 
@@ -1280,14 +1255,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;
 }
 
@@ -1305,7 +1278,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 +1291,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 +1308,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)
@@ -1605,7 +1574,7 @@ static GOptionEntry options[] = {
 int main(int argc, char *argv[])
 {
 	GOptionContext *context;
-	GError *err = NULL;
+	_cleanup_type_(GError) GError *err = NULL;
 	uint16_t sdp_mtu = 0;
 	uint32_t sdp_flags = 0;
 	int gdbus_flags = 0;
@@ -1618,7 +1587,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

[BlueZ 5/6] client: Use _cleanup_fd_ to simplify urandom access

[Bastien Nocera]

fd gets auto-closed before exiting the scope.
---
 client/mgmt.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/client/mgmt.c b/client/mgmt.c
index 50558a313866..e199c30540e4 100644
--- a/client/mgmt.c
+++ b/client/mgmt.c
@@ -2681,7 +2681,7 @@ static void cmd_privacy(int argc, char **argv)
 			return bt_shell_noninteractive_quit(EXIT_FAILURE);
 		}
 	} else {
-		int fd;
+		_cleanup_fd_ int fd = -1;
 
 		fd = open("/dev/urandom", O_RDONLY);
 		if (fd < 0) {
@@ -2691,11 +2691,8 @@ static void cmd_privacy(int argc, char **argv)
 
 		if (read(fd, cp.irk, sizeof(cp.irk)) != sizeof(cp.irk)) {
 			error("Reading from urandom failed");
-			close(fd);
 			return bt_shell_noninteractive_quit(EXIT_FAILURE);
 		}
-
-		close(fd);
 	}
 
 	if (send_cmd(mgmt, MGMT_OP_SET_PRIVACY, index, sizeof(cp), &cp,
-- 
2.54.0

[BlueZ 6/6] btattach: Use _cleanup_fd_ to simplify error paths

[Bastien Nocera]

Use _cleanup_fd_ and _steal_fd() to simplify error paths, and only
"steal" the file descriptor on success.
---
 tools/btattach.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/tools/btattach.c b/tools/btattach.c
index 5f7d19093698..4918c79f65c7 100644
--- a/tools/btattach.c
+++ b/tools/btattach.c
@@ -40,7 +40,8 @@
 static int open_serial(const char *path, unsigned int speed, bool flowctl)
 {
 	struct termios ti;
-	int fd, saved_ldisc, ldisc = N_HCI;
+	_cleanup_fd_ int fd = -1;
+	int saved_ldisc, ldisc = N_HCI;
 
 	fd = open(path, O_RDWR | O_NOCTTY);
 	if (fd < 0) {
@@ -50,13 +51,11 @@ static int open_serial(const char *path, unsigned int speed, bool flowctl)
 
 	if (tcflush(fd, TCIOFLUSH) < 0) {
 		perror("Failed to flush serial port");
-		close(fd);
 		return -1;
 	}
 
 	if (ioctl(fd, TIOCGETD, &saved_ldisc) < 0) {
 		perror("Failed get serial line discipline");
-		close(fd);
 		return -1;
 	}
 
@@ -73,19 +72,17 @@ static int open_serial(const char *path, unsigned int speed, bool flowctl)
 
 	if (tcsetattr(fd, TCSANOW, &ti) < 0) {
 		perror("Failed to set serial port settings");
-		close(fd);
 		return -1;
 	}
 
 	if (ioctl(fd, TIOCSETD, &ldisc) < 0) {
 		perror("Failed set serial line discipline");
-		close(fd);
 		return -1;
 	}
 
 	printf("Switched line discipline from %d to %d\n", saved_ldisc, ldisc);
 
-	return fd;
+	return _steal_fd_(fd);
 }
 
 static void local_version_callback(const void *data, uint8_t size,
@@ -99,7 +96,8 @@ static void local_version_callback(const void *data, uint8_t size,
 static int attach_proto(const char *path, unsigned int proto,
 			unsigned int speed, bool flowctl, unsigned int flags)
 {
-	int fd, dev_id;
+	_cleanup_fd_ int fd = -1;
+	int dev_id;
 
 	fd = open_serial(path, speed, flowctl);
 	if (fd < 0)
@@ -107,20 +105,17 @@ static int attach_proto(const char *path, unsigned int proto,
 
 	if (ioctl(fd, HCIUARTSETFLAGS, flags) < 0) {
 		perror("Failed to set flags");
-		close(fd);
 		return -1;
 	}
 
 	if (ioctl(fd, HCIUARTSETPROTO, proto) < 0) {
 		perror("Failed to set protocol");
-		close(fd);
 		return -1;
 	}
 
 	dev_id = ioctl(fd, HCIUARTGETDEVICE);
 	if (dev_id < 0) {
 		perror("Failed to get device id");
-		close(fd);
 		return -1;
 	}
 
@@ -140,7 +135,6 @@ static int attach_proto(const char *path, unsigned int proto,
 
 		if (!hci) {
 			fprintf(stderr, "Failed to open HCI user channel\n");
-			close(fd);
 			return -1;
 		}
 
@@ -149,7 +143,7 @@ static int attach_proto(const char *path, unsigned int proto,
 					(bt_hci_destroy_func_t) bt_hci_unref);
 	}
 
-	return fd;
+	return _steal_fd_(fd);
 }
 
 static void uart_callback(int fd, uint32_t events, void *user_data)
-- 
2.54.0