[dm-devel] [PATCH 00/16] multipath-tools: minor fixes and build improvements

[dm-devel] [PATCH 00/16] multipath-tools: minor fixes and build improvements

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hi Ben, hi Christophe,

here's a set of minor fixes for multipath-tools.

Regards
Martin

Martin Wilck (16):
  multipath-tools: Makefile: remove useless .PHONY targets
  multipath-tools: Makefile: fix install->all dependency
  multipath-tools: Makefile: remove dependency test -> test-progs
  multipath-tools: Makefile: run abidiff with --redundant flag
  libdmmp: Makefile: create man3dir
  tests/Makefile: use $(multipathdir)
  tests/Makefile: add library dependencies
  tests/Makefile: use symbolic links under tests/lib
  tests/Makefile: redirect program output into output file
  GitHub workflows: package shared objects in artifact
  libmultipath: replace close_fd() with cleanup_fd_ptr()
  libmultipath: cleanup_free_ptr(): avoid double free
  multipath: find_multipaths_check_timeout(): no need for pthread
    cleanup
  multipathd: fix segfault in cli_list_map_fmt()
  multipathd: fix broken pthread cleanup in
    fpin_fabric_notification_receiver()
  multipathd: Fix command completion in interactive mode

 .github/workflows/foreign.yaml    |  2 ++
 Makefile                          |  8 ++++----
 libdmmp/Makefile                  |  1 +
 libmpathutil/libmpathutil.version |  6 +++++-
 libmpathutil/util.c               | 19 +++++++++++++------
 libmpathutil/util.h               |  2 +-
 libmultipath/alias.c              |  4 ++--
 libmultipath/foreign/nvme.c       |  4 ++--
 libmultipath/sysfs.c              | 12 ++++++------
 libmultipath/wwids.c              |  8 ++++----
 multipath/main.c                  |  8 +++-----
 multipathd/cli.c                  |  2 ++
 multipathd/cli_handlers.c         |  2 +-
 multipathd/fpin_handlers.c        |  9 +++++----
 multipathd/main.c                 |  1 -
 tests/Makefile                    | 26 +++++++++++++-------------
 16 files changed, 64 insertions(+), 50 deletions(-)

-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 01/16] multipath-tools: Makefile: remove useless .PHONY targets

[mwilck]

From: Martin Wilck <mwilck@suse.com>

.PHONY is only necessary for targets that may match actually existing
files.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 5851149..74ab648 100644
--- a/Makefile
+++ b/Makefile
@@ -26,7 +26,7 @@ BUILDDIRS := $(LIB_BUILDDIRS) \
 
 BUILDDIRS.clean := $(BUILDDIRS:=.clean) tests.clean
 
-.PHONY:	$(BUILDDIRS) $(BUILDDIRS:=.uninstall) $(BUILDDIRS:=.install) $(BUILDDIRS:=.clean) $(LIB_BUILDDIRS:=.abi)
+.PHONY:	$(BUILDDIRS)
 
 all:	$(BUILDDIRS)
 
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 02/16] multipath-tools: Makefile: fix install->all dependency

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Rather than enforcing a "make all; make install" in the top directory,
make sure that the $DIR.install correctly depends on $DIR for all
subdirs. This speeds up compilation without breaking dependencies.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 74ab648..6ebd030 100644
--- a/Makefile
+++ b/Makefile
@@ -86,7 +86,7 @@ libmultipath/checkers.install \
 $(BUILDDIRS.clean):
 	$(MAKE) -C ${@:.clean=} clean
 
-$(BUILDDIRS:=.install): $(BUILDDIRS)
+%.install:	%
 	$(MAKE) -C ${@:.install=} install
 
 $(BUILDDIRS:=.uninstall):
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 03/16] multipath-tools: Makefile: remove dependency test -> test-progs

[mwilck]

From: Martin Wilck <mwilck@suse.com>

This dependency is already established in tests/Makefile. "test"
must depend on "all" though.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 6ebd030..1537cfd 100644
--- a/Makefile
+++ b/Makefile
@@ -101,7 +101,7 @@ uninstall: $(BUILDDIRS:=.uninstall)
 test-progs:	all
 	$(MAKE) -C tests progs
 
-test:	test-progs
+test:	all
 	$(MAKE) -C tests all
 
 valgrind-test:	all
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 04/16] multipath-tools: Makefile: run abidiff with --redundant flag

[mwilck]

From: Martin Wilck <mwilck@suse.com>

With --redundant, abidiff lists all changed functions, which may be
handy.
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1537cfd..27b4641 100644
--- a/Makefile
+++ b/Makefile
@@ -54,7 +54,7 @@ abi.tar.gz:	abi
 abi-test:	abi reference-abi $(wildcard abi/*.abi)
 	@err=0; \
 	for lib in abi/*.abi; do \
-	    diff=$$(abidiff "reference-$$lib" "$$lib") || { \
+	    diff=$$(abidiff --redundant "reference-$$lib" "$$lib") || { \
 	        err=1; \
 		echo "==== ABI differences in for $$lib ===="; \
 		echo "$$diff"; \
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 05/16] libdmmp: Makefile: create man3dir

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Without this "make install" may fail.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libdmmp/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libdmmp/Makefile b/libdmmp/Makefile
index 2e99b3e..e458925 100644
--- a/libdmmp/Makefile
+++ b/libdmmp/Makefile
@@ -45,6 +45,7 @@ install:
 		$(DESTDIR)$(pkgconfdir)/$(PKGFILE)
 	perl -i -pe 's|__INCLUDEDIR__|$(includedir)|g' \
 		$(DESTDIR)$(pkgconfdir)/$(PKGFILE)
+	$(INSTALL_PROGRAM) -d 755 $(DESTDIR)$(man3dir)
 	$(INSTALL_PROGRAM) -m 644 -t $(DESTDIR)$(man3dir) docs/man/*.3
 
 uninstall:
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 06/16] tests/Makefile: use $(multipathdir)

[mwilck]

From: Martin Wilck <mwilck@suse.com>

.. instead of hard-coded ../libmultipath.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index 9866962..e2793d5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -48,30 +48,30 @@ mpathvalid-test_FLAGS := -I$(mpathvaliddir)
 #    unit test file, e.g. "config-test.o", in XYZ-test_OBJDEPS
 # XYZ-test_LIBDEPS: Additional libs to link for this test
 
-dmevents-test_OBJDEPS = ../libmultipath/devmapper.o
+dmevents-test_OBJDEPS = $(multipathdir)/devmapper.o
 dmevents-test_LIBDEPS = -lpthread -ldevmapper -lurcu
 hwtable-test_TESTDEPS := test-lib.o
-hwtable-test_OBJDEPS := ../libmultipath/discovery.o ../libmultipath/blacklist.o \
-	../libmultipath/structs.o ../libmultipath/propsel.o
+hwtable-test_OBJDEPS := $(multipathdir)/discovery.o $(multipathdir)/blacklist.o \
+	$(multipathdir)/structs.o $(multipathdir)/propsel.o
 hwtable-test_LIBDEPS := -ludev -lpthread -ldl
 blacklist-test_TESTDEPS := test-log.o
-blacklist-test_OBJDEPS := ../libmultipath/blacklist.o
+blacklist-test_OBJDEPS := $(multipathdir)/blacklist.o
 blacklist-test_LIBDEPS := -ludev
-vpd-test_OBJDEPS :=  ../libmultipath/discovery.o
+vpd-test_OBJDEPS :=  $(multipathdir)/discovery.o
 vpd-test_LIBDEPS := -ludev -lpthread -ldl
 alias-test_TESTDEPS := test-log.o
 alias-test_LIBDEPS := -lpthread -ldl
-valid-test_OBJDEPS := ../libmultipath/valid.o ../libmultipath/discovery.o
+valid-test_OBJDEPS := $(multipathdir)/valid.o $(multipathdir)/discovery.o
 valid-test_LIBDEPS := -ludev -lpthread -ldl
 devt-test_LIBDEPS := -ludev
 mpathvalid-test_LIBDEPS := -ludev -lpthread -ldl
-mpathvalid-test_OBJDEPS := ../libmpathvalid/mpath_valid.o
+mpathvalid-test_OBJDEPS := $(mpathvaliddir)/mpath_valid.o
 ifneq ($(DIO_TEST_DEV),)
 directio-test_LIBDEPS := -laio
 endif
-strbuf-test_OBJDEPS := ../libmpathutil/strbuf.o
+strbuf-test_OBJDEPS := $(mpathutildir)/strbuf.o
 sysfs-test_TESTDEPS := test-log.o
-sysfs-test_OBJDEPS := ../libmultipath/sysfs.o ../libmpathutil/util.o
+sysfs-test_OBJDEPS := $(multipathdir)/sysfs.o $(mpathutildir)/util.o
 sysfs-test_LIBDEPS := -ludev -lpthread -ldl
 
 %.o: %.c
@@ -79,7 +79,7 @@ sysfs-test_LIBDEPS := -ludev -lpthread -ldl
 
 lib/libchecktur.so:
 	mkdir -p lib
-	ln ../libmultipath/*/*.so lib
+	ln $(multipathdir)/*/*.so lib
 
 %.out:	%-test lib/libchecktur.so
 	@echo == running $< ==
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 07/16] tests/Makefile: add library dependencies

[mwilck]

From: Martin Wilck <mwilck@suse.com>

The tests need to be re-run if changes have been made to libmultipath
and other libraries. Add dependencies to take care of that.
Also, the libmultipath.so.0 copy under tests must be rebuilt if anything
changes under libmultipath.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile b/tests/Makefile
index e2793d5..3d3b431 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -113,14 +113,14 @@ dep_clean:
 	@sed -n 's/^.*__wrap_\([a-zA-Z0-9_]*\).*$$/-Wl,--wrap=\1/p' $< | \
 		sort -u | tr '\n' ' ' >$@
 
-libmultipath.so.0:
+libmultipath.so.0: $(multipathdir)/libmultipath.so.0
 	make -C $(multipathdir) configdir=$(TESTDIR)/conf.d plugindir=$(TESTDIR)/lib test-lib
 
 # COLON will get expanded during second expansion below
 COLON:=:
 .SECONDEXPANSION:
 %-test:	%.o %.o.wrap $$($$@_OBJDEPS) $$($$@_TESTDEPS) $$($$@_TESTDEPS$$(COLON).o=.o.wrap) \
-		libmultipath.so.0 Makefile
+		libmultipath.so.0 $(mpathutildir)/libmpathutil.so.0 $(mpathcmddir)/libmpathcmd.so.0 Makefile
 	$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $< $($@_TESTDEPS) $($@_OBJDEPS) \
 		$(LIBDEPS) $($@_LIBDEPS) \
 		$(shell cat $<.wrap) $(foreach dep,$($@_TESTDEPS),$(shell cat $(dep).wrap))
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 08/16] tests/Makefile: use symbolic links under tests/lib

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hardlinks would not update if the .so files were rebuilt. Symlinks
work better for this purpose.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 3d3b431..7c42d06 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -79,7 +79,7 @@ sysfs-test_LIBDEPS := -ludev -lpthread -ldl
 
 lib/libchecktur.so:
 	mkdir -p lib
-	ln $(multipathdir)/*/*.so lib
+	cd lib && ln -s ../$(multipathdir)/*/*.so .
 
 %.out:	%-test lib/libchecktur.so
 	@echo == running $< ==
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 09/16] tests/Makefile: redirect program output into output file

[mwilck]

From: Martin Wilck <mwilck@suse.com>

The test program output is too verbose to read every time.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index 7c42d06..109ea75 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -83,7 +83,7 @@ lib/libchecktur.so:
 
 %.out:	%-test lib/libchecktur.so
 	@echo == running $< ==
-	@LD_LIBRARY_PATH=.:$(mpathutildir):$(mpathcmddir) ./$< >$@
+	@LD_LIBRARY_PATH=.:$(mpathutildir):$(mpathcmddir) ./$< >$@ 2>&1
 
 %.vgr:  %-test lib/libchecktur.so
 	@echo == running valgrind for $< ==
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 10/16] GitHub workflows: package shared objects in artifact

[mwilck]

From: Martin Wilck <mwilck@suse.com>

As we are now using symlinks under tests/lib, the .so files under
checkers, prioritizers, and foreign need to be packaged, too.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 .github/workflows/foreign.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.github/workflows/foreign.yaml b/.github/workflows/foreign.yaml
index 8fc6d49..bd4e9c1 100644
--- a/.github/workflows/foreign.yaml
+++ b/.github/workflows/foreign.yaml
@@ -32,6 +32,8 @@ jobs:
           tar cfv binaries.tar
           Makefile*
           libmpathcmd/*.so* libmultipath/*.so* libmpathutil/*.so*
+          libmultipath/checkers/*.so libmultipath/prioritizers/*.so
+          libmultipath/foreign/*.so
           tests/lib tests/*-test tests/Makefile tests/*.so*
       - uses: actions/upload-artifact@v1
         if: ${{ matrix.arch != '' && matrix.arch != '-i386' }}
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 11/16] libmultipath: replace close_fd() with cleanup_fd_ptr()

[mwilck]

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 --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;
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 12/16] libmultipath: cleanup_free_ptr(): avoid double free

[mwilck]

From: Martin Wilck <mwilck@suse.com>

... by nullifying the passed pointer after freeing it.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathutil/util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libmpathutil/util.c b/libmpathutil/util.c
index 1539738..6692ac2 100644
--- a/libmpathutil/util.c
+++ b/libmpathutil/util.c
@@ -391,8 +391,10 @@ void cleanup_free_ptr(void *arg)
 {
 	void **p = arg;
 
-	if (p && *p)
+	if (p && *p) {
 		free(*p);
+		*p = NULL;
+	}
 }
 
 void cleanup_fd_ptr(void *arg)
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 13/16] multipath: find_multipaths_check_timeout(): no need for pthread cleanup

[mwilck]

From: Martin Wilck <mwilck@suse.com>

multipath is not a multithreaded program, no pthread-cancel complexity
is necesssary here.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/main.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/multipath/main.c b/multipath/main.c
index fbff6b7..7b69a3c 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;
-	int fd = -1;
+	int fd;
 	int r, retries = 0;
 
 	clock_gettime(CLOCK_REALTIME, &now);
@@ -339,9 +339,8 @@ static int find_multipaths_check_timeout(const struct path *pp, long tmo,
 retry:
 	fd = open(path, O_RDONLY);
 	if (fd != -1) {
-		pthread_cleanup_push(cleanup_fd_ptr, &fd);
 		r = fstat(fd, &st);
-		pthread_cleanup_pop(1);
+		close(fd);
 
 	} else if (tmo > 0) {
 		if (errno == ENOENT)
@@ -355,7 +354,6 @@ retry:
 			return FIND_MULTIPATHS_ERROR;
 		};
 
-		pthread_cleanup_push(cleanup_fd_ptr, &fd);
 		/*
 		 * We just created the file. Set st_mtim to our desired
 		 * expiry time.
@@ -369,7 +367,7 @@ retry:
 				path, strerror(errno));
 		}
 		r = fstat(fd, &st);
-		pthread_cleanup_pop(1);
+		close(fd);
 	} else
 		return FIND_MULTIPATHS_NEVER;
 
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 14/16] multipathd: fix segfault in cli_list_map_fmt()

[mwilck]

From: Martin Wilck <mwilck@suse.com>

get_multipath_layout() must be passed an mpvec, not a pathvec.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/cli_handlers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index db4d441..3d39967 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -420,7 +420,7 @@ cli_list_map_fmt (void *v, struct strbuf *reply, void *data)
 
 	if ((width = alloc_multipath_layout()) == NULL)
 		return 1;
-	get_multipath_layout(vecs->pathvec, 1, width);
+	get_multipath_layout(vecs->mpvec, 1, width);
 	param = convert_dev(param, 0);
 	mpp = find_mp_by_str(vecs->mpvec, param);
 	if (!mpp)
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 15/16] multipathd: fix broken pthread cleanup in fpin_fabric_notification_receiver()

[mwilck]

From: Martin Wilck <mwilck@suse.com>

We were returning from a phtread-cancel block. That should be avoided.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/fpin_handlers.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/multipathd/fpin_handlers.c b/multipathd/fpin_handlers.c
index a7da2c9..03b2b9a 100644
--- a/multipathd/fpin_handlers.c
+++ b/multipathd/fpin_handlers.c
@@ -499,13 +499,14 @@ void *fpin_fabric_notification_receiver(__attribute__((unused))void *unused)
 	rcu_register_thread();
 
 	pthread_cleanup_push(receiver_cleanup_list, NULL);
+	pthread_cleanup_push(cleanup_fd_ptr, &fd);
+
 	fd = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_SCSITRANSPORT);
 	if (fd < 0) {
 		condlog(0, "fc socket error %d", fd);
-		return NULL;
+		goto out;
 	}
 
-	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;
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 16/16] multipathd: Fix command completion in interactive mode

[mwilck]

From: Martin Wilck <mwilck@suse.com>

The command completion never worked, because the handlers
array wasn't initialized in client mode.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/cli.c  | 2 ++
 multipathd/main.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/multipathd/cli.c b/multipathd/cli.c
index d1bfeee..5d25ddb 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -14,6 +14,7 @@
 
 #include "mpath_cmd.h"
 #include "cli.h"
+#include "cli_handlers.h"
 #include "debug.h"
 #include "strbuf.h"
 
@@ -451,6 +452,7 @@ cli_init (void) {
 	if (alloc_handlers())
 		return 1;
 
+	init_handler_callbacks();
 	return 0;
 }
 
diff --git a/multipathd/main.c b/multipathd/main.c
index 4a65359..ba52d39 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1797,7 +1797,6 @@ uxlsnrloop (void * ap)
 	/* Tell main thread that thread has started */
 	post_config_state(DAEMON_CONFIGURE);
 
-	init_handler_callbacks();
 	umask(077);
 
 	/*
-- 
2.37.1

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 00/16] multipath-tools: minor fixes and build improvements

[Benjamin Marzinski]

On Thu, Sep 01, 2022 at 06:09:36PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Hi Ben, hi Christophe,
> 
> here's a set of minor fixes for multipath-tools.
For the set:
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Regards
> Martin
> 
> Martin Wilck (16):
>   multipath-tools: Makefile: remove useless .PHONY targets
>   multipath-tools: Makefile: fix install->all dependency
>   multipath-tools: Makefile: remove dependency test -> test-progs
>   multipath-tools: Makefile: run abidiff with --redundant flag
>   libdmmp: Makefile: create man3dir
>   tests/Makefile: use $(multipathdir)
>   tests/Makefile: add library dependencies
>   tests/Makefile: use symbolic links under tests/lib
>   tests/Makefile: redirect program output into output file
>   GitHub workflows: package shared objects in artifact
>   libmultipath: replace close_fd() with cleanup_fd_ptr()
>   libmultipath: cleanup_free_ptr(): avoid double free
>   multipath: find_multipaths_check_timeout(): no need for pthread
>     cleanup
>   multipathd: fix segfault in cli_list_map_fmt()
>   multipathd: fix broken pthread cleanup in
>     fpin_fabric_notification_receiver()
>   multipathd: Fix command completion in interactive mode
> 
>  .github/workflows/foreign.yaml    |  2 ++
>  Makefile                          |  8 ++++----
>  libdmmp/Makefile                  |  1 +
>  libmpathutil/libmpathutil.version |  6 +++++-
>  libmpathutil/util.c               | 19 +++++++++++++------
>  libmpathutil/util.h               |  2 +-
>  libmultipath/alias.c              |  4 ++--
>  libmultipath/foreign/nvme.c       |  4 ++--
>  libmultipath/sysfs.c              | 12 ++++++------
>  libmultipath/wwids.c              |  8 ++++----
>  multipath/main.c                  |  8 +++-----
>  multipathd/cli.c                  |  2 ++
>  multipathd/cli_handlers.c         |  2 +-
>  multipathd/fpin_handlers.c        |  9 +++++----
>  multipathd/main.c                 |  1 -
>  tests/Makefile                    | 26 +++++++++++++-------------
>  16 files changed, 64 insertions(+), 50 deletions(-)
> 
> -- 
> 2.37.1
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel