[PATCH v2 00/39] multipath-tools series part I: minor changes

[PATCH v2 00/39] multipath-tools series part I: minor changes

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Hi Christophe, hi Ben,

This is part I of a larger patch series for multpath-tools I've been preparing.
It contains self-contained fixes and cleanups, and unit test additions.

This is v2 of the series I posted on 2020-07-09. To avoid spamming, I will
resend only the patches that have changed (including context changes).
I took care to make sure the numbering is preserved. The full series will
be available here:
https://github.com/mwilck/multipath-tools/tree/ups/submit-2008

There are tags in that repo for each part of the series.
This part is tagged "submit-200812-1".

It's based on 0.8.4, plus the following set of previously
submitted and reviewed patches (latest first):

 - [PATCH v2 0/8] multipath cleanups (v2 8-part series from Ben,
       without 7/8 "multipathd: unset mpp->hwe when removing map")
 - [PATCH v3] vector: return false if realloc fails in vector_alloc_slot func (Zhiqiang Liu)
 - libmultipath: free pgp if add_pathgroup fails in disassemble_map func (Zhiqiang Liu)
 - checker: add input check of state in checker_state_name func (Zhiqiang Liu)
 - libmultipath: fix null dereference in add (lixiaokeng)
 - tests/hwtable: fix a memory free in replicate_config (Zhiqiang Liu)
 - vector: fix upper boundary check of vector size in vector_del_slot (Zhiqiang Liu)
 - vector: add lower bound check of E in VECTOR_SLOT (Zhiqiang Liu)
 - devmapper: remove useless using of memset in dm_get_info func (Zhiqiang Liu)
 - libmultipath: fix a memory leak in dm_get_maps (lixiaokeng)
 - libmultipath: fix a memory leak in disassemble_status func (Zhiqiang Liu)
 - [PATCH 0/4] Fix segfault on access to mpp->hwe (4-part series, me)

(below is where the v1 series was based on)

 - libmultipath: add device to hwtable.c (Steve Schremmer)
 - [PATCH v3 0/7] Fix muitpath/multipathd flush issue (v3 7-part series, Ben)
 - [PATCH v2 0/4] misc patches (v2 4-part series, Ben)
 - multipath: Fix compiler warnings when built without systemd. (Marius Bakke)
 - [PATCH v2 0/3] multipath: change default devnode blacklist
   (v2 3-part series, Ben)
 - multipath: add "-e" option to enable foreign libraries (me)
 - libmultipath: set "enable_foreign" to NONE by default (me)
 - libmultipath: fix condlog NULL argument in uevent_get_env_var (Ben)
 - fix boolean value with json-c 0.14 (Christian Hesse) 
 - [PATCH v3 0/6] multipath: path validation library prep work
   (v3 6-part series, me)
 - [PATCH 0/2] More minor libmultipath fixes (2-part series, me)
 - [PATCH 00/11] Minor fixes for multipath-tools (11-part series, me)
 - libmpathpersist: depend on libmultipath (Christian Hesse)
 - [PATCH v2 0/2] multipath-tools: fixes for kpartx.rules and skip_kpartx
   (v2 2-part series, me)
 - libmultipath: allow force reload with no active paths (Ben)
 - libmutipath: don't close fd on dm_lib_release (Ben)-
 - libmultipath: assign variable to make gcc happy (Ben)
 - [PATCH v2 0/4] libmpathpersist allocation size fixes (v2 4-part series, me)

You can find a full tree of the code this is based on here:
https://github.com/openSUSE/multipath-tools/tree/upstream-queue

Regards, Martin

Changes v1 -> v2, as suggested by Ben Marzinski:

 *  8/35 "libmultipath: create bitfield abstraction"
    - return NULL for 0 bitfield size
    - fixed error handling for bf == NULL
    - removed unused find_first_set() and related tests
 * 12/35 "libmultipath: strlcpy()/strlcat(): use restrict qualifier"
    - unchanged, but resubmitting as it has no Reviewed-by: tag yet
      (corner case of unterminated "dst" still unresolved, but if at
       all, it should be changed in a separate patch).

Martin Wilck (35):
  multipath-tools tests/util: separate group for bitmask tests
  multipath-tools tests/directio: fix missing initializers
  tests: __wrap_dlog: use check_expected()
  multipath tools tests: add strchop() test
  libmultipath: improve strchop()
  multipath-tools tests: add test for devt2devname
  libmultipath: devt2devname(): simplify using libudev
  libmultipath: create bitfield abstraction
  libmultipath: use bitfields in group_by_match()
  libmultipath: util: constify function arguments
  multipath-tools tests: add unit tests for strlcat
  libmultipath: strlcpy()/strlcat(): use restrict qualifier
  libmultipath: constify blacklist code
  libmultipath: rlookup_binding(): remove newline in log message
  libmultipath: fix missing initializer warning from clang 3.9
  libmultipath: fix gcc -Wstringop-overflow warning
  libmultipath: remove uevent listener failback
  libmultipath: uevent: use static linkage where possible
  libmultipath: uevent: inline trivial functions
  libmultipath: decrease log level of "SCSI target" log message
  libmultipath: get_udev_uid(): more appropriate error code
  libmultipath: get_uid(): improve log message on udev failure
  libmultipath: make sysfs_pathinfo() static and use const
  libmultipath: pathinfo(): improve a log message
  libmultipath: pathinfo(): don't filter emtpy devnode names
  libmultipath: io_err_stat_handle_pathfail(): less error conditions
  libmultipath: improve libdm logging
  libmultipath: snprint_devices(): use udev_enumerate
  libmultipath: snprint_devices(): print hidden/foreign status
  libmultipath: alloc_path(): initialize pp->initialized
  libmultipath: alloc_path_with_pathinfo(): treat devname overflow as
    error
  libmultipath: log table params in addmap()
  multipathd: remove set_multipath_wwid()
  kpartx: print an error message if removing a partition fails
  kpartx: add missing newline

 kpartx/devmapper.c               |   2 +-
 kpartx/kpartx.c                  |   2 +
 libmultipath/alias.c             |   2 +-
 libmultipath/blacklist.c         |  34 +-
 libmultipath/blacklist.h         |  17 +-
 libmultipath/checkers/directio.c |   2 +-
 libmultipath/configure.c         |  11 +-
 libmultipath/defaults.h          |   2 +
 libmultipath/devmapper.c         |  27 +-
 libmultipath/discovery.c         |  30 +-
 libmultipath/dmparser.c          |   2 +-
 libmultipath/io_err_stat.c       |  25 +-
 libmultipath/parser.c            |   2 +-
 libmultipath/pgpolicies.c        |  12 +-
 libmultipath/print.c             |  90 ++---
 libmultipath/print.h             |   2 +-
 libmultipath/propsel.c           |   6 +
 libmultipath/structs.c           |   1 +
 libmultipath/uevent.c            | 324 ++---------------
 libmultipath/uevent.h            |  47 ++-
 libmultipath/util.c              | 168 ++++-----
 libmultipath/util.h              |  73 +++-
 multipathd/main.c                |  14 +-
 tests/Makefile                   |   3 +-
 tests/alias.c                    |   4 +-
 tests/devt.c                     | 192 ++++++++++
 tests/directio.c                 |  28 +-
 tests/test-log.c                 |   5 +-
 tests/uevent.c                   |   4 -
 tests/util.c                     | 586 ++++++++++++++++++++++++++++---
 30 files changed, 1081 insertions(+), 636 deletions(-)
 create mode 100644 tests/devt.c

-- 
2.26.2

[PATCH v2 08/35] libmultipath: create bitfield abstraction

[mwilck]

From: Martin Wilck <mwilck@suse.com>

In e32d521d ("libmultipath: coalesce_paths: fix size mismatch handling"),
we introduced simple bitmap handling functions. We can do better. This
patch introduces a bitfield type with overflow detection and a
find_first_set() method.

Use this in coalesce_paths(), and adapt the unit tests. Also, add
unit tests for "odd" bitfield sizes; so far we tested only multiples
of 64.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c |   9 +-
 libmultipath/util.c      |  22 ++++
 libmultipath/util.h      |  56 +++++++++-
 tests/util.c             | 231 ++++++++++++++++++++++++++++++++++-----
 4 files changed, 281 insertions(+), 37 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 96c7961..fe590f4 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1092,7 +1092,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 	vector pathvec = vecs->pathvec;
 	struct config *conf;
 	int allow_queueing;
-	uint64_t *size_mismatch_seen;
+	struct bitfield *size_mismatch_seen;
 
 	/* ignore refwwid if it's empty */
 	if (refwwid && !strlen(refwwid))
@@ -1106,8 +1106,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 
 	if (VECTOR_SIZE(pathvec) == 0)
 		return CP_OK;
-	size_mismatch_seen = calloc((VECTOR_SIZE(pathvec) - 1) / 64 + 1,
-				    sizeof(uint64_t));
+	size_mismatch_seen = alloc_bitfield(VECTOR_SIZE(pathvec));
 	if (size_mismatch_seen == NULL)
 		return CP_FAIL;
 
@@ -1131,7 +1130,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 		}
 
 		/* 2. if path already coalesced, or seen and discarded */
-		if (pp1->mpp || is_bit_set_in_array(k, size_mismatch_seen))
+		if (pp1->mpp || is_bit_set_in_bitfield(k, size_mismatch_seen))
 			continue;
 
 		/* 3. if path has disappeared */
@@ -1183,7 +1182,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 					"Discard", pp2->dev, pp2->size,
 					mpp->size);
 				mpp->action = ACT_REJECT;
-				set_bit_in_array(i, size_mismatch_seen);
+				set_bit_in_bitfield(i, size_mismatch_seen);
 			}
 		}
 		verify_paths(mpp, vecs);
diff --git a/libmultipath/util.c b/libmultipath/util.c
index 3c43f28..3e2708a 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -404,3 +404,25 @@ void close_fd(void *arg)
 {
 	close((long)arg);
 }
+
+struct bitfield *alloc_bitfield(unsigned int maxbit)
+{
+	unsigned int n;
+	struct bitfield *bf;
+
+	if (maxbit == 0) {
+		errno = EINVAL;
+		return NULL;
+	}
+
+	n = (maxbit - 1) / bits_per_slot + 1;
+	bf = calloc(1, sizeof(struct bitfield) + n * sizeof(bitfield_t));
+	if (bf)
+		bf->len = maxbit;
+	return bf;
+}
+
+void _log_bitfield_overflow(const char *f, unsigned int bit, unsigned int len)
+{
+	condlog(0, "%s: bitfield overflow: %u >= %u", f, bit, len);
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index df75c4f..7ed30c7 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -1,6 +1,9 @@
 #ifndef _UTIL_H
 #define _UTIL_H
 
+#include <stdlib.h>
+#include <string.h>
+#include <limits.h>
 #include <sys/types.h>
 /* for rlim_t */
 #include <sys/resource.h>
@@ -51,19 +54,60 @@ struct scandir_result {
 };
 void free_scandir_result(struct scandir_result *);
 
-static inline bool is_bit_set_in_array(unsigned int bit, const uint64_t *arr)
+/*
+ * ffsll() is also available on glibc < 2.27 if _GNU_SOURCE is defined.
+ * But relying on that would require that every program using this header file
+ * set _GNU_SOURCE during compilation, because otherwise the library and the
+ * program would use different types for bitfield_t, causing errors.
+ * That's too error prone, so if in doubt, use ffs().
+ */
+#if __GLIBC_PREREQ(2, 27)
+typedef unsigned long long int bitfield_t;
+#define _ffs(x) ffsll(x)
+#else
+typedef unsigned int bitfield_t;
+#define _ffs(x) ffs(x)
+#endif
+#define bits_per_slot (sizeof(bitfield_t) * CHAR_BIT)
+
+struct bitfield {
+	unsigned int len;
+	bitfield_t bits[];
+};
+
+struct bitfield *alloc_bitfield(unsigned int maxbit);
+
+void _log_bitfield_overflow(const char *f, unsigned int bit, unsigned int len);
+#define log_bitfield_overflow(bit, len) \
+	_log_bitfield_overflow(__func__, bit, len)
+
+static inline bool is_bit_set_in_bitfield(unsigned int bit,
+				       const struct bitfield *bf)
 {
-	return arr[bit / 64] & (1ULL << (bit % 64)) ? 1 : 0;
+	if (bit >= bf->len) {
+		log_bitfield_overflow(bit, bf->len);
+		return false;
+	}
+	return !!(bf->bits[bit / bits_per_slot] &
+		  (1ULL << (bit % bits_per_slot)));
 }
 
-static inline void set_bit_in_array(unsigned int bit, uint64_t *arr)
+static inline void set_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
 {
-	arr[bit / 64] |= (1ULL << (bit % 64));
+	if (bit >= bf->len) {
+		log_bitfield_overflow(bit, bf->len);
+		return;
+	}
+	bf->bits[bit / bits_per_slot] |= (1ULL << (bit % bits_per_slot));
 }
 
-static inline void clear_bit_in_array(unsigned int bit, uint64_t *arr)
+static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
 {
-	arr[bit / 64] &= ~(1ULL << (bit % 64));
+	if (bit >= bf->len) {
+		log_bitfield_overflow(bit, bf->len);
+		return;
+	}
+	bf->bits[bit / bits_per_slot] &= ~(1ULL << (bit % bits_per_slot));
 }
 
 #endif /* _UTIL_H */
diff --git a/tests/util.c b/tests/util.c
index 6d12fda..fa869cd 100644
--- a/tests/util.c
+++ b/tests/util.c
@@ -164,19 +164,25 @@ static int test_basenamecpy(void)
 
 static void test_bitmask_1(void **state)
 {
-	uint64_t arr[BITARR_SZ];
+	struct bitfield *bf;
+	uint64_t *arr;
 	int i, j, k, m, b;
 
-	memset(arr, 0, sizeof(arr));
+	bf = alloc_bitfield(BITARR_SZ * 64);
+	assert_non_null(bf);
+	assert_int_equal(bf->len, BITARR_SZ * 64);
+	arr = (uint64_t *)bf->bits;
 
 	for (j = 0; j < BITARR_SZ; j++) {
 		for (i = 0; i < 64; i++) {
 			b = 64 * j + i;
-			assert(!is_bit_set_in_array(b, arr));
-			set_bit_in_array(b, arr);
+			assert(!is_bit_set_in_bitfield(b, bf));
+			set_bit_in_bitfield(b, bf);
 			for (k = 0; k < BITARR_SZ; k++) {
+#if 0
 				printf("b = %d j = %d k = %d a = %"PRIx64"\n",
 				       b, j, k, arr[k]);
+#endif
 				if (k == j)
 					assert_int_equal(arr[j], 1ULL << i);
 				else
@@ -184,39 +190,44 @@ static void test_bitmask_1(void **state)
 			}
 			for (m = 0; m < 64; m++)
 				if (i == m)
-					assert(is_bit_set_in_array(64 * j + m,
-								   arr));
+					assert(is_bit_set_in_bitfield(64 * j + m,
+								      bf));
 				else
-					assert(!is_bit_set_in_array(64 * j + m,
-								    arr));
-			clear_bit_in_array(b, arr);
-			assert(!is_bit_set_in_array(b, arr));
+					assert(!is_bit_set_in_bitfield(64 * j + m,
+								       bf));
+			clear_bit_in_bitfield(b, bf);
+			assert(!is_bit_set_in_bitfield(b, bf));
 			for (k = 0; k < BITARR_SZ; k++)
 				assert_int_equal(arr[k], 0ULL);
 		}
 	}
+	free(bf);
 }
 
 static void test_bitmask_2(void **state)
 {
-	uint64_t arr[BITARR_SZ];
+	struct bitfield *bf;
+	uint64_t *arr;
 	int i, j, k, m, b;
 
-	memset(arr, 0, sizeof(arr));
+	bf = alloc_bitfield(BITARR_SZ * 64);
+	assert_non_null(bf);
+	assert_int_equal(bf->len, BITARR_SZ * 64);
+	arr = (uint64_t *)bf->bits;
 
 	for (j = 0; j < BITARR_SZ; j++) {
 		for (i = 0; i < 64; i++) {
 			b = 64 * j + i;
-			assert(!is_bit_set_in_array(b, arr));
-			set_bit_in_array(b, arr);
+			assert(!is_bit_set_in_bitfield(b, bf));
+			set_bit_in_bitfield(b, bf);
 			for (m = 0; m < 64; m++)
 				if (m <= i)
-					assert(is_bit_set_in_array(64 * j + m,
-								   arr));
+					assert(is_bit_set_in_bitfield(64 * j + m,
+								      bf));
 				else
-					assert(!is_bit_set_in_array(64 * j + m,
-								    arr));
-			assert(is_bit_set_in_array(b, arr));
+					assert(!is_bit_set_in_bitfield(64 * j + m,
+								       bf));
+			assert(is_bit_set_in_bitfield(b, bf));
 			for (k = 0; k < BITARR_SZ; k++) {
 				if (k < j || (k == j && i == 63))
 					assert_int_equal(arr[k], ~0ULL);
@@ -232,16 +243,16 @@ static void test_bitmask_2(void **state)
 	for (j = 0; j < BITARR_SZ; j++) {
 		for (i = 0; i < 64; i++) {
 			b = 64 * j + i;
-			assert(is_bit_set_in_array(b, arr));
-			clear_bit_in_array(b, arr);
+			assert(is_bit_set_in_bitfield(b, bf));
+			clear_bit_in_bitfield(b, bf);
 			for (m = 0; m < 64; m++)
 				if (m <= i)
-					assert(!is_bit_set_in_array(64 * j + m,
-								    arr));
+					assert(!is_bit_set_in_bitfield(64 * j + m,
+								       bf));
 				else
-					assert(is_bit_set_in_array(64 * j + m,
-								   arr));
-			assert(!is_bit_set_in_array(b, arr));
+					assert(is_bit_set_in_bitfield(64 * j + m,
+								      bf));
+			assert(!is_bit_set_in_bitfield(b, bf));
 			for (k = 0; k < BITARR_SZ; k++) {
 				if (k < j || (k == j && i == 63))
 					assert_int_equal(arr[k], 0ULL);
@@ -254,13 +265,181 @@ static void test_bitmask_2(void **state)
 			}
 		}
 	}
+	free(bf);
 }
 
+/*
+ *  Test operations on a 0-length bitfield
+ */
+static void test_bitmask_len_0(void **state)
+{
+	struct bitfield *bf;
+
+	bf = alloc_bitfield(0);
+	assert_null(bf);
+}
+
+static void _test_bitmask_small(unsigned int n)
+{
+	struct bitfield *bf;
+	uint64_t *arr;
+
+	assert(n <= 64);
+	assert(n >= 1);
+
+	bf = alloc_bitfield(n);
+	assert_non_null(bf);
+	assert_int_equal(bf->len, n);
+	arr = (uint64_t *)bf->bits;
+
+	assert_int_equal(*arr, 0);
+
+	set_bit_in_bitfield(n + 1, bf);
+	assert_int_equal(*arr, 0);
+
+	set_bit_in_bitfield(n, bf);
+	assert_int_equal(*arr, 0);
+
+	set_bit_in_bitfield(n - 1, bf);
+	assert_int_equal(*arr, 1ULL << (n - 1));
+
+	clear_bit_in_bitfield(n - 1, bf);
+	assert_int_equal(*arr, 0);
+
+	set_bit_in_bitfield(0, bf);
+	assert_int_equal(*arr, 1);
+
+	free(bf);
+}
+
+static void _test_bitmask_small_2(unsigned int n)
+{
+	struct bitfield *bf;
+	uint64_t *arr;
+
+	assert(n <= 128);
+	assert(n >= 65);
+
+	bf = alloc_bitfield(n);
+	assert_non_null(bf);
+	assert_int_equal(bf->len, n);
+	arr = (uint64_t *)bf->bits;
+
+	assert_int_equal(arr[0], 0);
+	assert_int_equal(arr[1], 0);
+
+	set_bit_in_bitfield(n + 1, bf);
+	assert_int_equal(arr[0], 0);
+	assert_int_equal(arr[1], 0);
+
+	set_bit_in_bitfield(n, bf);
+	assert_int_equal(arr[0], 0);
+	assert_int_equal(arr[1], 0);
+
+	set_bit_in_bitfield(n - 1, bf);
+	assert_int_equal(arr[0], 0);
+	assert_int_equal(arr[1], 1ULL << (n - 65));
+
+	set_bit_in_bitfield(0, bf);
+	assert_int_equal(arr[0], 1);
+	assert_int_equal(arr[1], 1ULL << (n - 65));
+
+	set_bit_in_bitfield(64, bf);
+	assert_int_equal(arr[0], 1);
+	assert_int_equal(arr[1], (1ULL << (n - 65)) | 1);
+
+	clear_bit_in_bitfield(0, bf);
+	assert_int_equal(arr[0], 0);
+	assert_int_equal(arr[1], (1ULL << (n - 65)) | 1);
+
+	free(bf);
+}
+
+static void test_bitmask_len_1(void **state)
+{
+	_test_bitmask_small(1);
+}
+
+static void test_bitmask_len_2(void **state)
+{
+	_test_bitmask_small(2);
+}
+
+static void test_bitmask_len_3(void **state)
+{
+	_test_bitmask_small(3);
+}
+
+static void test_bitmask_len_23(void **state)
+{
+	_test_bitmask_small(23);
+}
+
+static void test_bitmask_len_63(void **state)
+{
+	_test_bitmask_small(63);
+}
+
+static void test_bitmask_len_64(void **state)
+{
+	_test_bitmask_small(63);
+}
+
+static void test_bitmask_len_65(void **state)
+{
+	_test_bitmask_small_2(65);
+}
+
+static void test_bitmask_len_66(void **state)
+{
+	_test_bitmask_small_2(66);
+}
+
+static void test_bitmask_len_67(void **state)
+{
+	_test_bitmask_small_2(67);
+}
+
+static void test_bitmask_len_103(void **state)
+{
+	_test_bitmask_small_2(103);
+}
+
+static void test_bitmask_len_126(void **state)
+{
+	_test_bitmask_small_2(126);
+}
+
+static void test_bitmask_len_127(void **state)
+{
+	_test_bitmask_small_2(127);
+}
+
+static void test_bitmask_len_128(void **state)
+{
+	_test_bitmask_small_2(128);
+}
+
+
 static int test_bitmasks(void)
 {
 	const struct CMUnitTest tests[] = {
 		cmocka_unit_test(test_bitmask_1),
 		cmocka_unit_test(test_bitmask_2),
+		cmocka_unit_test(test_bitmask_len_0),
+		cmocka_unit_test(test_bitmask_len_1),
+		cmocka_unit_test(test_bitmask_len_2),
+		cmocka_unit_test(test_bitmask_len_3),
+		cmocka_unit_test(test_bitmask_len_23),
+		cmocka_unit_test(test_bitmask_len_63),
+		cmocka_unit_test(test_bitmask_len_64),
+		cmocka_unit_test(test_bitmask_len_65),
+		cmocka_unit_test(test_bitmask_len_66),
+		cmocka_unit_test(test_bitmask_len_67),
+		cmocka_unit_test(test_bitmask_len_103),
+		cmocka_unit_test(test_bitmask_len_126),
+		cmocka_unit_test(test_bitmask_len_127),
+		cmocka_unit_test(test_bitmask_len_128),
 	};
 	return cmocka_run_group_tests(tests, NULL, NULL);
 }
-- 
2.28.0

[PATCH v2 12/35] libmultipath: strlcpy()/strlcat(): use restrict qualifier

[mwilck]

From: Martin Wilck <mwilck@suse.com>

Also remove the redundant local variables. It's not necessary to
make "restrict" work, but it makes the intention more clear.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/util.c | 28 ++++++++++++----------------
 libmultipath/util.h |  4 ++--
 2 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/libmultipath/util.c b/libmultipath/util.c
index 526869e..207e63c 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -113,46 +113,42 @@ get_word (const char *sentence, char **word)
 	return skip + len;
 }
 
-size_t strlcpy(char *dst, const char *src, size_t size)
+size_t strlcpy(char * restrict dst, const char * restrict src, size_t size)
 {
 	size_t bytes = 0;
-	char *q = dst;
-	const char *p = src;
 	char ch;
 
-	while ((ch = *p++)) {
-		if (bytes+1 < size)
-			*q++ = ch;
+	while ((ch = *src++)) {
+		if (bytes + 1 < size)
+			*dst++ = ch;
 		bytes++;
 	}
 
 	/* If size == 0 there is no space for a final null... */
 	if (size)
-		*q = '\0';
+		*dst = '\0';
 	return bytes;
 }
 
-size_t strlcat(char *dst, const char *src, size_t size)
+size_t strlcat(char * restrict dst, const char * restrict src, size_t size)
 {
 	size_t bytes = 0;
-	char *q = dst;
-	const char *p = src;
 	char ch;
 
-	while (bytes < size && *q) {
-		q++;
+	while (bytes < size && *dst) {
+		dst++;
 		bytes++;
 	}
 	if (bytes == size)
 		return (bytes + strlen(src));
 
-	while ((ch = *p++)) {
-		if (bytes+1 < size)
-		*q++ = ch;
+	while ((ch = *src++)) {
+		if (bytes + 1 < size)
+			*dst++ = ch;
 		bytes++;
 	}
 
-	*q = '\0';
+	*dst = '\0';
 	return bytes;
 }
 
diff --git a/libmultipath/util.h b/libmultipath/util.h
index a4f7c0a..52aa559 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -15,8 +15,8 @@ int basenamecpy (const char *src, char *dst, size_t size);
 int filepresent (const char *run);
 char *get_next_string(char **temp, const char *split_char);
 int get_word (const char * sentence, char ** word);
-size_t strlcpy(char *dst, const char *src, size_t size);
-size_t strlcat(char *dst, const char *src, size_t size);
+size_t strlcpy(char * restrict dst, const char * restrict src, size_t size);
+size_t strlcat(char * restrict dst, const char * restrict src, size_t size);
 int devt2devname (char *, int, const char *);
 dev_t parse_devt(const char *dev_t);
 char *convert_dev(char *dev, int is_path_device);
-- 
2.28.0

Re: [PATCH v2 08/35] libmultipath: create bitfield abstraction

[Benjamin Marzinski]

On Wed, Aug 12, 2020 at 01:32:31PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> In e32d521d ("libmultipath: coalesce_paths: fix size mismatch handling"),
> we introduced simple bitmap handling functions. We can do better. This
> patch introduces a bitfield type with overflow detection and a
> find_first_set() method.
> 
> Use this in coalesce_paths(), and adapt the unit tests. Also, add
> unit tests for "odd" bitfield sizes; so far we tested only multiples
> of 64.
> 
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/configure.c |   9 +-
>  libmultipath/util.c      |  22 ++++
>  libmultipath/util.h      |  56 +++++++++-
>  tests/util.c             | 231 ++++++++++++++++++++++++++++++++++-----
>  4 files changed, 281 insertions(+), 37 deletions(-)
> 
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 96c7961..fe590f4 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -1092,7 +1092,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
>  	vector pathvec = vecs->pathvec;
>  	struct config *conf;
>  	int allow_queueing;
> -	uint64_t *size_mismatch_seen;
> +	struct bitfield *size_mismatch_seen;
>  
>  	/* ignore refwwid if it's empty */
>  	if (refwwid && !strlen(refwwid))
> @@ -1106,8 +1106,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
>  
>  	if (VECTOR_SIZE(pathvec) == 0)
>  		return CP_OK;
> -	size_mismatch_seen = calloc((VECTOR_SIZE(pathvec) - 1) / 64 + 1,
> -				    sizeof(uint64_t));
> +	size_mismatch_seen = alloc_bitfield(VECTOR_SIZE(pathvec));
>  	if (size_mismatch_seen == NULL)
>  		return CP_FAIL;
>  
> @@ -1131,7 +1130,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
>  		}
>  
>  		/* 2. if path already coalesced, or seen and discarded */
> -		if (pp1->mpp || is_bit_set_in_array(k, size_mismatch_seen))
> +		if (pp1->mpp || is_bit_set_in_bitfield(k, size_mismatch_seen))
>  			continue;
>  
>  		/* 3. if path has disappeared */
> @@ -1183,7 +1182,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
>  					"Discard", pp2->dev, pp2->size,
>  					mpp->size);
>  				mpp->action = ACT_REJECT;
> -				set_bit_in_array(i, size_mismatch_seen);
> +				set_bit_in_bitfield(i, size_mismatch_seen);
>  			}
>  		}
>  		verify_paths(mpp, vecs);
> diff --git a/libmultipath/util.c b/libmultipath/util.c
> index 3c43f28..3e2708a 100644
> --- a/libmultipath/util.c
> +++ b/libmultipath/util.c
> @@ -404,3 +404,25 @@ void close_fd(void *arg)
>  {
>  	close((long)arg);
>  }
> +
> +struct bitfield *alloc_bitfield(unsigned int maxbit)
> +{
> +	unsigned int n;
> +	struct bitfield *bf;
> +
> +	if (maxbit == 0) {
> +		errno = EINVAL;
> +		return NULL;
> +	}
> +
> +	n = (maxbit - 1) / bits_per_slot + 1;
> +	bf = calloc(1, sizeof(struct bitfield) + n * sizeof(bitfield_t));
> +	if (bf)
> +		bf->len = maxbit;
> +	return bf;
> +}
> +
> +void _log_bitfield_overflow(const char *f, unsigned int bit, unsigned int len)
> +{
> +	condlog(0, "%s: bitfield overflow: %u >= %u", f, bit, len);
> +}
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index df75c4f..7ed30c7 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> @@ -1,6 +1,9 @@
>  #ifndef _UTIL_H
>  #define _UTIL_H
>  
> +#include <stdlib.h>
> +#include <string.h>
> +#include <limits.h>
>  #include <sys/types.h>
>  /* for rlim_t */
>  #include <sys/resource.h>
> @@ -51,19 +54,60 @@ struct scandir_result {
>  };
>  void free_scandir_result(struct scandir_result *);
>  
> -static inline bool is_bit_set_in_array(unsigned int bit, const uint64_t *arr)
> +/*
> + * ffsll() is also available on glibc < 2.27 if _GNU_SOURCE is defined.
> + * But relying on that would require that every program using this header file
> + * set _GNU_SOURCE during compilation, because otherwise the library and the
> + * program would use different types for bitfield_t, causing errors.
> + * That's too error prone, so if in doubt, use ffs().
> + */
> +#if __GLIBC_PREREQ(2, 27)
> +typedef unsigned long long int bitfield_t;
> +#define _ffs(x) ffsll(x)
> +#else
> +typedef unsigned int bitfield_t;
> +#define _ffs(x) ffs(x)
> +#endif
> +#define bits_per_slot (sizeof(bitfield_t) * CHAR_BIT)
> +
> +struct bitfield {
> +	unsigned int len;
> +	bitfield_t bits[];
> +};
> +
> +struct bitfield *alloc_bitfield(unsigned int maxbit);
> +
> +void _log_bitfield_overflow(const char *f, unsigned int bit, unsigned int len);
> +#define log_bitfield_overflow(bit, len) \
> +	_log_bitfield_overflow(__func__, bit, len)
> +
> +static inline bool is_bit_set_in_bitfield(unsigned int bit,
> +				       const struct bitfield *bf)
>  {
> -	return arr[bit / 64] & (1ULL << (bit % 64)) ? 1 : 0;
> +	if (bit >= bf->len) {
> +		log_bitfield_overflow(bit, bf->len);
> +		return false;
> +	}
> +	return !!(bf->bits[bit / bits_per_slot] &
> +		  (1ULL << (bit % bits_per_slot)));
>  }
>  
> -static inline void set_bit_in_array(unsigned int bit, uint64_t *arr)
> +static inline void set_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
>  {
> -	arr[bit / 64] |= (1ULL << (bit % 64));
> +	if (bit >= bf->len) {
> +		log_bitfield_overflow(bit, bf->len);
> +		return;
> +	}
> +	bf->bits[bit / bits_per_slot] |= (1ULL << (bit % bits_per_slot));
>  }
>  
> -static inline void clear_bit_in_array(unsigned int bit, uint64_t *arr)
> +static inline void clear_bit_in_bitfield(unsigned int bit, struct bitfield *bf)
>  {
> -	arr[bit / 64] &= ~(1ULL << (bit % 64));
> +	if (bit >= bf->len) {
> +		log_bitfield_overflow(bit, bf->len);
> +		return;
> +	}
> +	bf->bits[bit / bits_per_slot] &= ~(1ULL << (bit % bits_per_slot));
>  }
>  
>  #endif /* _UTIL_H */
> diff --git a/tests/util.c b/tests/util.c
> index 6d12fda..fa869cd 100644
> --- a/tests/util.c
> +++ b/tests/util.c
> @@ -164,19 +164,25 @@ static int test_basenamecpy(void)
>  
>  static void test_bitmask_1(void **state)
>  {
> -	uint64_t arr[BITARR_SZ];
> +	struct bitfield *bf;
> +	uint64_t *arr;
>  	int i, j, k, m, b;
>  
> -	memset(arr, 0, sizeof(arr));
> +	bf = alloc_bitfield(BITARR_SZ * 64);
> +	assert_non_null(bf);
> +	assert_int_equal(bf->len, BITARR_SZ * 64);
> +	arr = (uint64_t *)bf->bits;
>  
>  	for (j = 0; j < BITARR_SZ; j++) {
>  		for (i = 0; i < 64; i++) {
>  			b = 64 * j + i;
> -			assert(!is_bit_set_in_array(b, arr));
> -			set_bit_in_array(b, arr);
> +			assert(!is_bit_set_in_bitfield(b, bf));
> +			set_bit_in_bitfield(b, bf);
>  			for (k = 0; k < BITARR_SZ; k++) {
> +#if 0
>  				printf("b = %d j = %d k = %d a = %"PRIx64"\n",
>  				       b, j, k, arr[k]);
> +#endif
>  				if (k == j)
>  					assert_int_equal(arr[j], 1ULL << i);
>  				else
> @@ -184,39 +190,44 @@ static void test_bitmask_1(void **state)
>  			}
>  			for (m = 0; m < 64; m++)
>  				if (i == m)
> -					assert(is_bit_set_in_array(64 * j + m,
> -								   arr));
> +					assert(is_bit_set_in_bitfield(64 * j + m,
> +								      bf));
>  				else
> -					assert(!is_bit_set_in_array(64 * j + m,
> -								    arr));
> -			clear_bit_in_array(b, arr);
> -			assert(!is_bit_set_in_array(b, arr));
> +					assert(!is_bit_set_in_bitfield(64 * j + m,
> +								       bf));
> +			clear_bit_in_bitfield(b, bf);
> +			assert(!is_bit_set_in_bitfield(b, bf));
>  			for (k = 0; k < BITARR_SZ; k++)
>  				assert_int_equal(arr[k], 0ULL);
>  		}
>  	}
> +	free(bf);
>  }
>  
>  static void test_bitmask_2(void **state)
>  {
> -	uint64_t arr[BITARR_SZ];
> +	struct bitfield *bf;
> +	uint64_t *arr;
>  	int i, j, k, m, b;
>  
> -	memset(arr, 0, sizeof(arr));
> +	bf = alloc_bitfield(BITARR_SZ * 64);
> +	assert_non_null(bf);
> +	assert_int_equal(bf->len, BITARR_SZ * 64);
> +	arr = (uint64_t *)bf->bits;
>  
>  	for (j = 0; j < BITARR_SZ; j++) {
>  		for (i = 0; i < 64; i++) {
>  			b = 64 * j + i;
> -			assert(!is_bit_set_in_array(b, arr));
> -			set_bit_in_array(b, arr);
> +			assert(!is_bit_set_in_bitfield(b, bf));
> +			set_bit_in_bitfield(b, bf);
>  			for (m = 0; m < 64; m++)
>  				if (m <= i)
> -					assert(is_bit_set_in_array(64 * j + m,
> -								   arr));
> +					assert(is_bit_set_in_bitfield(64 * j + m,
> +								      bf));
>  				else
> -					assert(!is_bit_set_in_array(64 * j + m,
> -								    arr));
> -			assert(is_bit_set_in_array(b, arr));
> +					assert(!is_bit_set_in_bitfield(64 * j + m,
> +								       bf));
> +			assert(is_bit_set_in_bitfield(b, bf));
>  			for (k = 0; k < BITARR_SZ; k++) {
>  				if (k < j || (k == j && i == 63))
>  					assert_int_equal(arr[k], ~0ULL);
> @@ -232,16 +243,16 @@ static void test_bitmask_2(void **state)
>  	for (j = 0; j < BITARR_SZ; j++) {
>  		for (i = 0; i < 64; i++) {
>  			b = 64 * j + i;
> -			assert(is_bit_set_in_array(b, arr));
> -			clear_bit_in_array(b, arr);
> +			assert(is_bit_set_in_bitfield(b, bf));
> +			clear_bit_in_bitfield(b, bf);
>  			for (m = 0; m < 64; m++)
>  				if (m <= i)
> -					assert(!is_bit_set_in_array(64 * j + m,
> -								    arr));
> +					assert(!is_bit_set_in_bitfield(64 * j + m,
> +								       bf));
>  				else
> -					assert(is_bit_set_in_array(64 * j + m,
> -								   arr));
> -			assert(!is_bit_set_in_array(b, arr));
> +					assert(is_bit_set_in_bitfield(64 * j + m,
> +								      bf));
> +			assert(!is_bit_set_in_bitfield(b, bf));
>  			for (k = 0; k < BITARR_SZ; k++) {
>  				if (k < j || (k == j && i == 63))
>  					assert_int_equal(arr[k], 0ULL);
> @@ -254,13 +265,181 @@ static void test_bitmask_2(void **state)
>  			}
>  		}
>  	}
> +	free(bf);
>  }
>  
> +/*
> + *  Test operations on a 0-length bitfield
> + */
> +static void test_bitmask_len_0(void **state)
> +{
> +	struct bitfield *bf;
> +
> +	bf = alloc_bitfield(0);
> +	assert_null(bf);
> +}
> +
> +static void _test_bitmask_small(unsigned int n)
> +{
> +	struct bitfield *bf;
> +	uint64_t *arr;
> +
> +	assert(n <= 64);
> +	assert(n >= 1);
> +
> +	bf = alloc_bitfield(n);
> +	assert_non_null(bf);
> +	assert_int_equal(bf->len, n);
> +	arr = (uint64_t *)bf->bits;
> +
> +	assert_int_equal(*arr, 0);
> +
> +	set_bit_in_bitfield(n + 1, bf);
> +	assert_int_equal(*arr, 0);
> +
> +	set_bit_in_bitfield(n, bf);
> +	assert_int_equal(*arr, 0);
> +
> +	set_bit_in_bitfield(n - 1, bf);
> +	assert_int_equal(*arr, 1ULL << (n - 1));
> +
> +	clear_bit_in_bitfield(n - 1, bf);
> +	assert_int_equal(*arr, 0);
> +
> +	set_bit_in_bitfield(0, bf);
> +	assert_int_equal(*arr, 1);
> +
> +	free(bf);
> +}
> +
> +static void _test_bitmask_small_2(unsigned int n)
> +{
> +	struct bitfield *bf;
> +	uint64_t *arr;
> +
> +	assert(n <= 128);
> +	assert(n >= 65);
> +
> +	bf = alloc_bitfield(n);
> +	assert_non_null(bf);
> +	assert_int_equal(bf->len, n);
> +	arr = (uint64_t *)bf->bits;
> +
> +	assert_int_equal(arr[0], 0);
> +	assert_int_equal(arr[1], 0);
> +
> +	set_bit_in_bitfield(n + 1, bf);
> +	assert_int_equal(arr[0], 0);
> +	assert_int_equal(arr[1], 0);
> +
> +	set_bit_in_bitfield(n, bf);
> +	assert_int_equal(arr[0], 0);
> +	assert_int_equal(arr[1], 0);
> +
> +	set_bit_in_bitfield(n - 1, bf);
> +	assert_int_equal(arr[0], 0);
> +	assert_int_equal(arr[1], 1ULL << (n - 65));
> +
> +	set_bit_in_bitfield(0, bf);
> +	assert_int_equal(arr[0], 1);
> +	assert_int_equal(arr[1], 1ULL << (n - 65));
> +
> +	set_bit_in_bitfield(64, bf);
> +	assert_int_equal(arr[0], 1);
> +	assert_int_equal(arr[1], (1ULL << (n - 65)) | 1);
> +
> +	clear_bit_in_bitfield(0, bf);
> +	assert_int_equal(arr[0], 0);
> +	assert_int_equal(arr[1], (1ULL << (n - 65)) | 1);
> +
> +	free(bf);
> +}
> +
> +static void test_bitmask_len_1(void **state)
> +{
> +	_test_bitmask_small(1);
> +}
> +
> +static void test_bitmask_len_2(void **state)
> +{
> +	_test_bitmask_small(2);
> +}
> +
> +static void test_bitmask_len_3(void **state)
> +{
> +	_test_bitmask_small(3);
> +}
> +
> +static void test_bitmask_len_23(void **state)
> +{
> +	_test_bitmask_small(23);
> +}
> +
> +static void test_bitmask_len_63(void **state)
> +{
> +	_test_bitmask_small(63);
> +}
> +
> +static void test_bitmask_len_64(void **state)
> +{
> +	_test_bitmask_small(63);
> +}
> +
> +static void test_bitmask_len_65(void **state)
> +{
> +	_test_bitmask_small_2(65);
> +}
> +
> +static void test_bitmask_len_66(void **state)
> +{
> +	_test_bitmask_small_2(66);
> +}
> +
> +static void test_bitmask_len_67(void **state)
> +{
> +	_test_bitmask_small_2(67);
> +}
> +
> +static void test_bitmask_len_103(void **state)
> +{
> +	_test_bitmask_small_2(103);
> +}
> +
> +static void test_bitmask_len_126(void **state)
> +{
> +	_test_bitmask_small_2(126);
> +}
> +
> +static void test_bitmask_len_127(void **state)
> +{
> +	_test_bitmask_small_2(127);
> +}
> +
> +static void test_bitmask_len_128(void **state)
> +{
> +	_test_bitmask_small_2(128);
> +}
> +
> +
>  static int test_bitmasks(void)
>  {
>  	const struct CMUnitTest tests[] = {
>  		cmocka_unit_test(test_bitmask_1),
>  		cmocka_unit_test(test_bitmask_2),
> +		cmocka_unit_test(test_bitmask_len_0),
> +		cmocka_unit_test(test_bitmask_len_1),
> +		cmocka_unit_test(test_bitmask_len_2),
> +		cmocka_unit_test(test_bitmask_len_3),
> +		cmocka_unit_test(test_bitmask_len_23),
> +		cmocka_unit_test(test_bitmask_len_63),
> +		cmocka_unit_test(test_bitmask_len_64),
> +		cmocka_unit_test(test_bitmask_len_65),
> +		cmocka_unit_test(test_bitmask_len_66),
> +		cmocka_unit_test(test_bitmask_len_67),
> +		cmocka_unit_test(test_bitmask_len_103),
> +		cmocka_unit_test(test_bitmask_len_126),
> +		cmocka_unit_test(test_bitmask_len_127),
> +		cmocka_unit_test(test_bitmask_len_128),
>  	};
>  	return cmocka_run_group_tests(tests, NULL, NULL);
>  }
> -- 
> 2.28.0

Re: [PATCH v2 12/35] libmultipath: strlcpy()/strlcat(): use restrict qualifier

[Benjamin Marzinski]

On Wed, Aug 12, 2020 at 01:32:32PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> Also remove the redundant local variables. It's not necessary to
> make "restrict" work, but it makes the intention more clear.
> 

Clearly, the way you wrote strlcat agrees with other implementations,
so it's fine.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/util.c | 28 ++++++++++++----------------
>  libmultipath/util.h |  4 ++--
>  2 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/libmultipath/util.c b/libmultipath/util.c
> index 526869e..207e63c 100644
> --- a/libmultipath/util.c
> +++ b/libmultipath/util.c
> @@ -113,46 +113,42 @@ get_word (const char *sentence, char **word)
>  	return skip + len;
>  }
>  
> -size_t strlcpy(char *dst, const char *src, size_t size)
> +size_t strlcpy(char * restrict dst, const char * restrict src, size_t size)
>  {
>  	size_t bytes = 0;
> -	char *q = dst;
> -	const char *p = src;
>  	char ch;
>  
> -	while ((ch = *p++)) {
> -		if (bytes+1 < size)
> -			*q++ = ch;
> +	while ((ch = *src++)) {
> +		if (bytes + 1 < size)
> +			*dst++ = ch;
>  		bytes++;
>  	}
>  
>  	/* If size == 0 there is no space for a final null... */
>  	if (size)
> -		*q = '\0';
> +		*dst = '\0';
>  	return bytes;
>  }
>  
> -size_t strlcat(char *dst, const char *src, size_t size)
> +size_t strlcat(char * restrict dst, const char * restrict src, size_t size)
>  {
>  	size_t bytes = 0;
> -	char *q = dst;
> -	const char *p = src;
>  	char ch;
>  
> -	while (bytes < size && *q) {
> -		q++;
> +	while (bytes < size && *dst) {
> +		dst++;
>  		bytes++;
>  	}
>  	if (bytes == size)
>  		return (bytes + strlen(src));
>  
> -	while ((ch = *p++)) {
> -		if (bytes+1 < size)
> -		*q++ = ch;
> +	while ((ch = *src++)) {
> +		if (bytes + 1 < size)
> +			*dst++ = ch;
>  		bytes++;
>  	}
>  
> -	*q = '\0';
> +	*dst = '\0';
>  	return bytes;
>  }
>  
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index a4f7c0a..52aa559 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> @@ -15,8 +15,8 @@ int basenamecpy (const char *src, char *dst, size_t size);
>  int filepresent (const char *run);
>  char *get_next_string(char **temp, const char *split_char);
>  int get_word (const char * sentence, char ** word);
> -size_t strlcpy(char *dst, const char *src, size_t size);
> -size_t strlcat(char *dst, const char *src, size_t size);
> +size_t strlcpy(char * restrict dst, const char * restrict src, size_t size);
> +size_t strlcat(char * restrict dst, const char * restrict src, size_t size);
>  int devt2devname (char *, int, const char *);
>  dev_t parse_devt(const char *dev_t);
>  char *convert_dev(char *dev, int is_path_device);
> -- 
> 2.28.0

Re: [dm-devel] [PATCH v2 08/35] libmultipath: create bitfield abstraction

[Xose Vazquez Perez]

On 8/12/20 1:32 PM, mwilck@suse.com wrote:

> From: Martin Wilck <mwilck@suse.com>
> 
> In e32d521d ("libmultipath: coalesce_paths: fix size mismatch handling"),
> we introduced simple bitmap handling functions. We can do better. This
> patch introduces a bitfield type with overflow detection and a
> find_first_set() method.
> 
> Use this in coalesce_paths(), and adapt the unit tests. Also, add
> unit tests for "odd" bitfield sizes; so far we tested only multiples
> of 64.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>   libmultipath/configure.c |   9 +-
>   libmultipath/util.c      |  22 ++++
>   libmultipath/util.h      |  56 +++++++++-
>   tests/util.c             | 231 ++++++++++++++++++++++++++++++++++-----
>   4 files changed, 281 insertions(+), 37 deletions(-)
> [...]
> diff --git a/libmultipath/util.h b/libmultipath/util.h
> index df75c4f..7ed30c7 100644
> --- a/libmultipath/util.h
> +++ b/libmultipath/util.h
> [...]
> -static inline bool is_bit_set_in_array(unsigned int bit, const uint64_t *arr)
> +/*
> + * ffsll() is also available on glibc < 2.27 if _GNU_SOURCE is defined.
> + * But relying on that would require that every program using this header file
> + * set _GNU_SOURCE during compilation, because otherwise the library and the
> + * program would use different types for bitfield_t, causing errors.
> + * That's too error prone, so if in doubt, use ffs().
> + */
> +#if __GLIBC_PREREQ(2, 27)
> +typedef unsigned long long int bitfield_t;
> +#define _ffs(x) ffsll(x)
> +#else
> +typedef unsigned int bitfield_t;
> +#define _ffs(x) ffs(x)
> +#endif
> [...]

musl-libc shows a warning(missing binary operator):

make[1]: Entering directory 'multipath-tools/libmultipath'
[...]
musl-gcc --std=gnu99  -O2 -g -fstack-protector-strong --param=ssp-buffer-size=4 -Werror -Wall -Wextra -Wformat=2 -Werror=implicit-int -Werror=implicit-function-declaration -Werror=format-security 
-Wno-clobbered -Wno-error=clobbered -Werror=cast-qual -Werror=discarded-qualifiers -pipe -DBIN_DIR=\"/sbin\" -DLIB_STRING=\"lib64\" -DRUN_DIR=\"run\" -MMD -MP -fPIC -I../libmpathcmd 
-I../libmpathpersist -I../libmultipath/nvme -DUSE_SYSTEMD=246 -DLIBDM_API_FLUSH -D_GNU_SOURCE -DLIBDM_API_COOKIE -DLIBUDEV_API_RECVBUF -DLIBDM_API_DEFERRED -DLIBDM_API_HOLD_CONTROL 
-Wp,-D_FORTIFY_SOURCE=2  -c -o devmapper.o devmapper.c
In file included from devmapper.c:19:
util.h:65:19: error: missing binary operator before token "("
    65 | #if __GLIBC_PREREQ(2, 27)
       |                   ^
make[1]: *** [../Makefile.inc:138: devmapper.o] Error 1

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

Re: [dm-devel] [PATCH v2 08/35] libmultipath: create bitfield abstraction

[Martin Wilck]

On Tue, 2020-10-27 at 12:41 +0100, Xose Vazquez Perez wrote:
> 
> musl-libc shows a warning(missing binary operator):

Thanks for pointing this out. There are more issues with musl libc,
in particular with the unit tests.

I'll send patches as soon as I've figured it out.

@Christophe, please don't tag the current code just yet. We should
include these fixes.

Regards
Martin





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