diff mbox series

[11/16] libmultipath: replace close_fd() with cleanup_fd_ptr()

Message ID 20220901160952.2167-12-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: christophe varoqui
Headers show
Series multipath-tools: minor fixes and build improvements | expand

Commit Message

Martin Wilck Sept. 1, 2022, 4:09 p.m. UTC 
From: Martin Wilck <mwilck@suse.com>

This is a nicer API without ugly casts, and less likely to close
valid file descriptors accidentally. Also, it can be used for
both pthread_cleanup_push and __attribute__((cleanup)).

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/libmpathutil.version |  6 +++++-
 libmpathutil/util.c               | 15 ++++++++++-----
 libmpathutil/util.h               |  2 +-
 libmultipath/alias.c              |  4 ++--
 libmultipath/foreign/nvme.c       |  4 ++--
 libmultipath/sysfs.c              | 12 ++++++------
 libmultipath/wwids.c              |  8 ++++----
 multipath/main.c                  |  6 +++---
 multipathd/fpin_handlers.c        |  6 +++---
 9 files changed, 36 insertions(+), 27 deletions(-)
diff mbox series

Patch

diff --git a/libmpathutil/libmpathutil.version b/libmpathutil/libmpathutil.version
index f81fb36..95b169d 100644
--- a/libmpathutil/libmpathutil.version
+++ b/libmpathutil/libmpathutil.version
@@ -39,7 +39,6 @@  global:
 	cleanup_charp;
 	cleanup_mutex;
 	cleanup_ucharp;
-	close_fd;
 	convert_dev;
 	dlog;
 	fill_strbuf;
@@ -121,3 +120,8 @@  LIBMPATHUTIL_1.0 {
 	vector_move_up;
 	vector_sort;
 };
+
+LIBMPATHUTIL_1.1 {
+global:
+	cleanup_fd_ptr;
+} LIBMPATHUTIL_1.0;
diff --git a/libmpathutil/util.c b/libmpathutil/util.c
index 6979e74..1539738 100644
--- a/libmpathutil/util.c
+++ b/libmpathutil/util.c
@@ -387,11 +387,6 @@  void free_scandir_result(struct scandir_result *res)
 	free(res->di);
 }
 
-void close_fd(void *arg)
-{
-	close((long)arg);
-}
-
 void cleanup_free_ptr(void *arg)
 {
 	void **p = arg;
@@ -400,6 +395,16 @@  void cleanup_free_ptr(void *arg)
 		free(*p);
 }
 
+void cleanup_fd_ptr(void *arg)
+{
+	int *fd = arg;
+
+	if (*fd >= 0) {
+		close(*fd);
+		*fd = -1;
+	}
+}
+
 void cleanup_mutex(void *arg)
 {
 	pthread_mutex_unlock(arg);
diff --git a/libmpathutil/util.h b/libmpathutil/util.h
index bede49d..7e34c56 100644
--- a/libmpathutil/util.h
+++ b/libmpathutil/util.h
@@ -46,7 +46,7 @@  int should_exit(void);
 #define pthread_cleanup_push_cast(f, arg)		\
 	pthread_cleanup_push(((void (*)(void *))&f), (arg))
 
-void close_fd(void *arg);
+void cleanup_fd_ptr(void *arg);
 void cleanup_free_ptr(void *arg);
 void cleanup_mutex(void *arg);
 
diff --git a/libmultipath/alias.c b/libmultipath/alias.c
index af3e24f..0520122 100644
--- a/libmultipath/alias.c
+++ b/libmultipath/alias.c
@@ -573,7 +573,7 @@  static int fix_bindings_file(const struct config *conf,
 			     const Bindings *bindings)
 {
 	int rc;
-	long fd;
+	int fd = -1;
 	char tempname[PATH_MAX];
 	mode_t old_umask;
 
@@ -586,7 +586,7 @@  static int fix_bindings_file(const struct config *conf,
 		return -1;
 	}
 	umask(old_umask);
-	pthread_cleanup_push(close_fd, (void*)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 	rc = write_bindings_file(bindings, fd);
 	pthread_cleanup_pop(1);
 	if (rc == -1) {
diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
index 9a05b33..edc9bd8 100644
--- a/libmultipath/foreign/nvme.c
+++ b/libmultipath/foreign/nvme.c
@@ -599,7 +599,7 @@  static void test_ana_support(struct nvme_map *map, struct udev_device *ctl)
 {
 	const char *dev_t;
 	char sys_path[64];
-	long fd;
+	int fd = -1;
 	int rc;
 
 	if (map->ana_supported != YNU_UNDEF)
@@ -615,7 +615,7 @@  static void test_ana_support(struct nvme_map *map, struct udev_device *ctl)
 		return;
 	}
 
-	pthread_cleanup_push(close_fd, (void *)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 	rc = nvme_id_ctrl_ana(fd, NULL);
 	if (rc < 0)
 		condlog(2, "%s: error in nvme_id_ctrl: %s", __func__,
diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 6494638..afde849 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -49,7 +49,7 @@  static ssize_t __sysfs_attr_get_value(struct udev_device *dev, const char *attr_
 {
 	const char *syspath;
 	char devpath[PATH_MAX];
-	long fd;
+	int fd = -1;
 	ssize_t size = -1;
 
 	if (!dev || !attr_name || !value || !value_len) {
@@ -74,7 +74,7 @@  static ssize_t __sysfs_attr_get_value(struct udev_device *dev, const char *attr_
 			__func__, devpath, strerror(errno));
 		return -errno;
 	}
-	pthread_cleanup_push(close_fd, (void *)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 
 	size = read(fd, value, value_len);
 	if (size < 0) {
@@ -114,7 +114,7 @@  ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 {
 	const char *syspath;
 	char devpath[PATH_MAX];
-	long fd;
+	int fd = -1;
 	ssize_t size = -1;
 
 	if (!dev || !attr_name || !value || !value_len) {
@@ -140,7 +140,7 @@  ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 			__func__, devpath, strerror(errno));
 		return -errno;
 	}
-	pthread_cleanup_push(close_fd, (void *)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 
 	size = write(fd, value, value_len);
 	if (size < 0) {
@@ -272,7 +272,7 @@  bool sysfs_is_multipathed(struct path *pp, bool set_wwid)
 	sr.n = r;
 	pthread_cleanup_push_cast(free_scandir_result, &sr);
 	for (i = 0; i < r && !found; i++) {
-		long fd;
+		int fd = -1;
 		int nr;
 		char uuid[WWID_SIZE + UUID_PREFIX_LEN];
 
@@ -286,7 +286,7 @@  bool sysfs_is_multipathed(struct path *pp, bool set_wwid)
 			continue;
 		}
 
-		pthread_cleanup_push(close_fd, (void *)fd);
+		pthread_cleanup_push(cleanup_fd_ptr, &fd);
 		nr = read(fd, uuid, sizeof(uuid));
 		if (nr > (int)UUID_PREFIX_LEN &&
 		    !memcmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN)) {
diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index 61d9c39..89bb60c 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -90,7 +90,7 @@  int
 replace_wwids(vector mp)
 {
 	int i, can_write;
-	long fd;
+	int fd = -1;
 	struct multipath * mpp;
 	size_t len;
 	int ret = -1;
@@ -103,7 +103,7 @@  replace_wwids(vector mp)
 	if (fd < 0)
 		goto out;
 
-	pthread_cleanup_push(close_fd, (void*)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 	if (!can_write) {
 		condlog(0, "cannot replace wwids. wwids file is read-only");
 		goto out_file;
@@ -196,7 +196,7 @@  do_remove_wwid(int fd, char *str) {
 
 int
 remove_wwid(char *wwid) {
-	long fd;
+	int fd = -1;
 	int len, can_write;
 	char *str;
 	int ret = -1;
@@ -226,7 +226,7 @@  remove_wwid(char *wwid) {
 		goto out;
 	}
 
-	pthread_cleanup_push(close_fd, (void*)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 	if (!can_write) {
 		ret = -1;
 		condlog(0, "cannot remove wwid. wwids file is read-only");
diff --git a/multipath/main.c b/multipath/main.c
index 8e5154a..fbff6b7 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -321,7 +321,7 @@  static int find_multipaths_check_timeout(const struct path *pp, long tmo,
 	char path[PATH_MAX];
 	struct timespec now, ftimes[2], tdiff;
 	struct stat st;
-	long fd;
+	int fd = -1;
 	int r, retries = 0;
 
 	clock_gettime(CLOCK_REALTIME, &now);
@@ -339,7 +339,7 @@  static int find_multipaths_check_timeout(const struct path *pp, long tmo,
 retry:
 	fd = open(path, O_RDONLY);
 	if (fd != -1) {
-		pthread_cleanup_push(close_fd, (void *)fd);
+		pthread_cleanup_push(cleanup_fd_ptr, &fd);
 		r = fstat(fd, &st);
 		pthread_cleanup_pop(1);
 
@@ -355,7 +355,7 @@  retry:
 			return FIND_MULTIPATHS_ERROR;
 		};
 
-		pthread_cleanup_push(close_fd, (void *)fd);
+		pthread_cleanup_push(cleanup_fd_ptr, &fd);
 		/*
 		 * We just created the file. Set st_mtim to our desired
 		 * expiry time.
diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c
index 0019572..a7da2c9 100644
--- a/multipathd/fpin_handlers.c
+++ b/multipathd/fpin_handlers.c
@@ -488,7 +488,7 @@  static void receiver_cleanup_list(__attribute__((unused)) void *arg)
 void *fpin_fabric_notification_receiver(__attribute__((unused))void *unused)
 {
 	int ret;
-	long fd;
+	int fd = -1;
 	uint32_t els_cmd;
 	struct fc_nl_event *fc_event = NULL;
 	struct sockaddr_nl fc_local;
@@ -501,11 +501,11 @@  void *fpin_fabric_notification_receiver(__attribute__((unused))void *unused)
 	pthread_cleanup_push(receiver_cleanup_list, NULL);
 	fd = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_SCSITRANSPORT);
 	if (fd < 0) {
-		condlog(0, "fc socket error %ld", fd);
+		condlog(0, "fc socket error %d", fd);
 		return NULL;
 	}
 
-	pthread_cleanup_push(close_fd, (void *)fd);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
 	memset(&fc_local, 0, sizeof(fc_local));
 	fc_local.nl_family = AF_NETLINK;
 	fc_local.nl_groups = ~0;