[PATCH 0/6] Cleanup

[PATCH 0/6] Cleanup

[Zdenek Kabelac]

Nice-to-have patch set.
Constains some warning fixes for various compilation builds which I'm
using on my machine (so not covering all configure options)
Should be easy to overview.

Zdenek Kabelac (6):
  Unreleased pool is INTERNAL_ERROR
  Remove double  brackets
  Add void and few more warning
  Add attribute printf
  Fix signes warnings
  const fixes

 daemons/clvmd/clvmd.c         |   11 ++++++-----
 daemons/clvmd/lvm-functions.c |    8 ++++----
 daemons/dmeventd/dmeventd.c   |    4 ++--
 lib/activate/dev_manager.c    |    2 +-
 lib/device/dev-io.c           |    2 +-
 lib/metadata/lv_manip.c       |    6 +++---
 lib/metadata/pv_manip.c       |    2 +-
 lib/misc/lvm-globals.c        |   24 ++++++++++++------------
 libdm/libdm-common.c          |    7 +++++--
 libdm/libdm-deptree.c         |    4 ++--
 libdm/mm/pool.c               |    2 +-
 tools/dmsetup.c               |    9 +++++----
 tools/pvmove.c                |    4 ++--
 tools/vgreduce.c              |    4 ++--
 14 files changed, 47 insertions(+), 42 deletions(-)

-- 
1.7.4.1

[PATCH 1/6] Unreleased pool is INTERNAL_ERROR

[Zdenek Kabelac]

It's better for catching bugs in test-suite.
(As currently one test does not pass without noticing.)

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 libdm/mm/pool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/libdm/mm/pool.c b/libdm/mm/pool.c
index 825f7ca..0e2444d 100644
--- a/libdm/mm/pool.c
+++ b/libdm/mm/pool.c
@@ -64,7 +64,6 @@ void dm_pools_check_leaks(void)
 	if (dm_list_empty(&_dm_pools))
 		return;
 
-	log_error("You have a memory leak (not released memory pool):");
 	dm_list_iterate_items(p, &_dm_pools) {
 #ifdef DEBUG_POOL
 		log_error(" [%p] %s (%u bytes)",
@@ -74,4 +73,5 @@ void dm_pools_check_leaks(void)
 		log_error(" [%p] %s", p, p->name);
 #endif
 	}
+	log_error(INTERNAL_ERROR "Unreleased memory pool(s) found.");
 }
-- 
1.7.4.1

[PATCH 2/6] Remove double brackets

[Zdenek Kabelac]

These are not needed - and clang is able to give some note about those.
So clean them.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/device/dev-io.c     |    2 +-
 lib/metadata/lv_manip.c |    6 +++---
 lib/metadata/pv_manip.c |    2 +-
 libdm/libdm-deptree.c   |    4 ++--
 tools/pvmove.c          |    4 ++--
 tools/vgreduce.c        |    4 ++--
 6 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/device/dev-io.c b/lib/device/dev-io.c
index 887d732..3de6cd1 100644
--- a/lib/device/dev-io.c
+++ b/lib/device/dev-io.c
@@ -126,7 +126,7 @@ static int _get_block_size(struct device *dev, unsigned int *size)
 {
 	const char *name = dev_name(dev);
 
-	if ((dev->block_size == -1)) {
+	if (dev->block_size == -1) {
 		if (ioctl(dev_fd(dev), BLKBSZGET, &dev->block_size) < 0) {
 			log_sys_error("ioctl BLKBSZGET", name);
 			return 0;
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index 015c481..a07421c 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -736,11 +736,11 @@ static void _init_alloc_parms(struct alloc_handle *ah, struct alloc_parms *alloc
 
 	/* Are there any preceding segments we must follow on from? */
 	if (alloc_parms->prev_lvseg) {
-		if ((alloc_parms->alloc == ALLOC_CONTIGUOUS))
+		if (alloc_parms->alloc == ALLOC_CONTIGUOUS)
 			alloc_parms->flags |= A_CONTIGUOUS;
-		else if ((alloc_parms->alloc == ALLOC_CLING))
+		else if (alloc_parms->alloc == ALLOC_CLING)
 			alloc_parms->flags |= A_CLING;
-		else if ((alloc_parms->alloc == ALLOC_CLING_BY_TAGS)) {
+		else if (alloc_parms->alloc == ALLOC_CLING_BY_TAGS) {
 			alloc_parms->flags |= A_CLING;
 			alloc_parms->flags |= A_CLING_BY_TAGS;
 		}
diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index cb37745..fdba91d 100644
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -468,7 +468,7 @@ int pv_resize(struct physical_volume *pv,
 			return 0;
 		}
 
-		if ((new_pe_count == old_pe_count)) {
+		if (new_pe_count == old_pe_count) {
 			pv->pe_count = old_pe_count;
 			log_verbose("No change to size of physical volume %s.",
 				    pv_dev_name(pv));
diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c
index 2449240..345257b 100644
--- a/libdm/libdm-deptree.c
+++ b/libdm/libdm-deptree.c
@@ -329,13 +329,13 @@ static void _remove_from_bottomlevel(struct dm_tree_node *node)
 static int _link_tree_nodes(struct dm_tree_node *parent, struct dm_tree_node *child)
 {
 	/* Don't link to root node if child already has a parent */
-	if ((parent == &parent->dtree->root)) {
+	if (parent == &parent->dtree->root) {
 		if (dm_tree_node_num_children(child, 1))
 			return 1;
 	} else
 		_remove_from_toplevel(child);
 
-	if ((child == &child->dtree->root)) {
+	if (child == &child->dtree->root) {
 		if (dm_tree_node_num_children(parent, 0))
 			return 1;
 	} else
diff --git a/tools/pvmove.c b/tools/pvmove.c
index c661e62..965a9eb 100644
--- a/tools/pvmove.c
+++ b/tools/pvmove.c
@@ -121,7 +121,7 @@ static struct dm_list *_get_allocatable_pvs(struct cmd_context *cmd, int argc,
 		}
 
 		/* Remove PV if full */
-		if ((pvl->pv->pe_count == pvl->pv->pe_alloc_count))
+		if (pvl->pv->pe_count == pvl->pv->pe_alloc_count)
 			dm_list_del(&pvl->list);
 	}
 
@@ -202,7 +202,7 @@ static struct logical_volume *_set_up_pvmove_lv(struct cmd_context *cmd,
 	/* Find segments to be moved and set up mirrors */
 	dm_list_iterate_items(lvl, &vg->lvs) {
 		lv = lvl->lv;
-		if ((lv == lv_mirr))
+		if (lv == lv_mirr)
 			continue;
 		if (lv_name) {
 			if (strcmp(lv->name, lv_name))
diff --git a/tools/vgreduce.c b/tools/vgreduce.c
index 1cabc8d..5697947 100644
--- a/tools/vgreduce.c
+++ b/tools/vgreduce.c
@@ -287,8 +287,8 @@ static int _make_vg_consistent(struct cmd_context *cmd, struct volume_group *vg)
 			if (mirrored_seg->log_lv) {
 				dm_list_iterate_items(seg, &mirrored_seg->log_lv->segments) {
 					/* FIXME: The second test shouldn't be required */
-					if ((seg->segtype ==
-					     get_segtype_from_string(vg->cmd, "error"))) {
+					if (seg->segtype ==
+					    get_segtype_from_string(vg->cmd, "error")) {
 						log_print("The log device for %s/%s has failed.",
 							  vg->name, mirrored_seg->lv->name);
 						remove_log = 1;
-- 
1.7.4.1

[PATCH 3/6] Add void and few more warning

[Zdenek Kabelac]

Fix -Wold-style-definition  warning

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 daemons/clvmd/clvmd.c         |   11 ++++++-----
 daemons/clvmd/lvm-functions.c |    8 ++++----
 daemons/dmeventd/dmeventd.c   |    3 +--
 lib/activate/dev_manager.c    |    2 +-
 lib/misc/lvm-globals.c        |   24 ++++++++++++------------
 5 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/daemons/clvmd/clvmd.c b/daemons/clvmd/clvmd.c
index 361fc62..fb3cc4f 100644
--- a/daemons/clvmd/clvmd.c
+++ b/daemons/clvmd/clvmd.c
@@ -609,7 +609,7 @@ int main(int argc, char *argv[])
 
 /* Called when the GuLM cluster layer has completed initialisation.
    We send the version message */
-void clvmd_cluster_init_completed()
+void clvmd_cluster_init_completed(void)
 {
 	send_version_message();
 }
@@ -802,9 +802,10 @@ static void request_timed_out(struct local_client *client)
 /* This is where the real work happens */
 static void main_loop(int local_sock, int cmd_timeout)
 {
+	sigset_t ss;
+
 	DEBUGLOG("Using timeout of %d seconds\n", cmd_timeout);
 
-	sigset_t ss;
 	sigemptyset(&ss);
 	sigaddset(&ss, SIGINT);
 	sigaddset(&ss, SIGTERM);
@@ -1826,7 +1827,7 @@ static void free_reply(struct local_client *client)
 }
 
 /* Send our version number to the cluster */
-static void send_version_message()
+static void send_version_message(void)
 {
 	char message[sizeof(struct clvm_header) + sizeof(int) * 3];
 	struct clvm_header *msg = (struct clvm_header *) message;
@@ -2056,7 +2057,7 @@ static void close_local_sock(int local_socket)
 }
 
 /* Open the local socket, that's the one we talk to libclvm down */
-static int open_local_sock()
+static int open_local_sock(void)
 {
 	int local_socket = -1;
 	struct sockaddr_un sockaddr;
@@ -2218,7 +2219,7 @@ static if_type_t parse_cluster_interface(char *ifname)
  * only called if the command-line option is not present, and if it fails
  * we still try the interfaces in order.
  */
-static if_type_t get_cluster_type()
+static if_type_t get_cluster_type(void)
 {
 #ifdef HAVE_COROSYNC_CONFDB_H
 	confdb_handle_t handle;
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 3cf7c50..166cd7f 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -147,7 +147,7 @@ static const char *decode_flags(unsigned char flags)
 	return buf;
 }
 
-char *get_last_lvm_error()
+char *get_last_lvm_error(void)
 {
 	return last_error;
 }
@@ -194,7 +194,7 @@ static int get_current_lock(char *resource)
 }
 
 
-void init_lvhash()
+void init_lvhash(void)
 {
 	/* Create hash table for keeping LV locks & status */
 	lv_hash = dm_hash_create(1024);
@@ -203,7 +203,7 @@ void init_lvhash()
 }
 
 /* Called at shutdown to tidy the lockspace */
-void destroy_lvhash()
+void destroy_lvhash(void)
 {
 	struct dm_hash_node *v;
 	struct lv_info *lvi;
@@ -621,7 +621,7 @@ int do_check_lvm1(const char *vgname)
 	return status == 1 ? 0 : EBUSY;
 }
 
-int do_refresh_cache()
+int do_refresh_cache(void)
 {
 	DEBUGLOG("Refreshing context\n");
 	log_notice("Refreshing context");
diff --git a/daemons/dmeventd/dmeventd.c b/daemons/dmeventd/dmeventd.c
index b30c204..a64295f 100644
--- a/daemons/dmeventd/dmeventd.c
+++ b/daemons/dmeventd/dmeventd.c
@@ -1225,6 +1225,7 @@ static void _init_fifos(struct dm_event_fifos *fifos)
 static int _open_fifos(struct dm_event_fifos *fifos)
 {
 	int orig_errno;
+	struct stat st;
 
 	/* Create client fifo. */
 	(void) dm_prepare_selinux_context(fifos->client_path, S_IFIFO);
@@ -1248,8 +1249,6 @@ static int _open_fifos(struct dm_event_fifos *fifos)
 
 	(void) dm_prepare_selinux_context(NULL, 0);
 
-	struct stat st;
-
 	/* Warn about wrong permissions if applicable */
 	if ((!stat(fifos->client_path, &st)) && (st.st_mode & 0777) != 0600)
 		syslog(LOG_WARNING, "Fixing wrong permissions on %s",
diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c
index cb041c6..9f4b2da 100644
--- a/lib/activate/dev_manager.c
+++ b/lib/activate/dev_manager.c
@@ -1104,7 +1104,7 @@ static char *_add_error_device(struct dev_manager *dm, struct dm_tree *dtree,
 	char errid[32];
 	struct dm_tree_node *node;
 	struct lv_segment *seg_i;
-	int segno = -1, i = 0;;
+	int segno = -1, i = 0;
 	uint64_t size = seg->len * seg->lv->vg->extent_size;
 
 	dm_list_iterate_items(seg_i, &seg->lv->segments) {
diff --git a/lib/misc/lvm-globals.c b/lib/misc/lvm-globals.c
index 7adb5e3..8325dec 100644
--- a/lib/misc/lvm-globals.c
+++ b/lib/misc/lvm-globals.c
@@ -147,7 +147,7 @@ void set_sysfs_dir_path(const char *path)
 	_sysfs_dir_path[sizeof(_sysfs_dir_path) - 1] = '\0';
 }
 
-const char *log_command_name()
+const char *log_command_name(void)
 {
 	if (!_log_cmd_name)
 		return "";
@@ -165,42 +165,42 @@ int error_message_produced(void)
 	return _error_message_produced;
 }
 
-int test_mode()
+int test_mode(void)
 {
 	return _test;
 }
 
-int md_filtering()
+int md_filtering(void)
 {
 	return _md_filtering;
 }
 
-int pvmove_mode()
+int pvmove_mode(void)
 {
 	return _pvmove;
 }
 
-int full_scan_done()
+int full_scan_done(void)
 {
 	return _full_scan_done;
 }
 
-int trust_cache()
+int trust_cache(void)
 {
 	return _trust_cache;
 }
 
-int background_polling()
+int background_polling(void)
 {
 	return _background_polling;
 }
 
-int ignorelockingfailure()
+int ignorelockingfailure(void)
 {
 	return _ignorelockingfailure;
 }
 
-int security_level()
+int security_level(void)
 {
 	return _security_level;
 }
@@ -225,12 +225,12 @@ void init_debug(int level)
 	_debug_level = level;
 }
 
-int verbose_level()
+int verbose_level(void)
 {
 	return _verbose_level;
 }
 
-int debug_level()
+int debug_level(void)
 {
 	return _debug_level;
 }
@@ -245,7 +245,7 @@ int udev_checking(void)
 	return _udev_checking;
 }
 
-const char *sysfs_dir_path()
+const char *sysfs_dir_path(void)
 {
 	return _sysfs_dir_path;
 }
-- 
1.7.4.1

[PATCH 4/6] Add attribute printf

[Zdenek Kabelac]

gcc suggested to add these __atributes__ for instrumentation
of printf parametr - so making it a bit happier.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 daemons/dmeventd/dmeventd.c |    1 +
 libdm/libdm-common.c        |    3 +++
 tools/dmsetup.c             |    1 +
 3 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/daemons/dmeventd/dmeventd.c b/daemons/dmeventd/dmeventd.c
index a64295f..fe5f019 100644
--- a/daemons/dmeventd/dmeventd.c
+++ b/daemons/dmeventd/dmeventd.c
@@ -567,6 +567,7 @@ static void _unregister_for_timeout(struct thread_status *thread)
 	pthread_mutex_unlock(&_timeout_mutex);
 }
 
+__attribute__((format(printf, 4, 5)))
 static void _no_intr_log(int level, const char *file, int line,
 			const char *f, ...)
 {
diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
index c5780e7..7cce3a7 100644
--- a/libdm/libdm-common.c
+++ b/libdm/libdm-common.c
@@ -78,6 +78,7 @@ static int _udev_checking = 1;
  * function.
  */
 
+__attribute__((format(printf, 5, 0)))
 static void _default_log_line(int level,
 	    const char *file __attribute__((unused)),
 	    int line __attribute__((unused)), int dm_errno, 
@@ -101,6 +102,7 @@ static void _default_log_line(int level,
 		fprintf(use_stderr ? stderr : stdout, "\n");
 }
 
+__attribute__((format(printf, 5, 6)))
 static void _default_log_with_errno(int level,
 	    const char *file __attribute__((unused)),
 	    int line __attribute__((unused)), int dm_errno, 
@@ -113,6 +115,7 @@ static void _default_log_with_errno(int level,
 	va_end(ap);
 }
 
+__attribute__((format(printf, 4, 5)))
 static void _default_log(int level, const char *file,
 			 int line, const char *f, ...)
 {
diff --git a/tools/dmsetup.c b/tools/dmsetup.c
index fb71350..5455369 100644
--- a/tools/dmsetup.c
+++ b/tools/dmsetup.c
@@ -1050,6 +1050,7 @@ static int _udevreleasecookie(CMD_ARGS)
 	return dm_udev_wait(_udev_cookie);
 }
 
+__attribute__((format(printf, 1, 2)))
 static char _yes_no_prompt(const char *prompt, ...)
 {
 	int c = 0, ret = 0;
-- 
1.7.4.1

[PATCH 5/6] Fix signes warnings

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 libdm/libdm-common.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
index 7cce3a7..bbf0aa4 100644
--- a/libdm/libdm-common.c
+++ b/libdm/libdm-common.c
@@ -797,7 +797,7 @@ static int _other_node_ops(node_op_t type)
 	int i;
 
 	for (i = 0; i < NUM_NODES; i++)
-		if (type != i && _count_node_ops[i])
+		if ((int) type != i && _count_node_ops[i])
 			return 1;
 	return 0;
 }
@@ -959,7 +959,7 @@ int dm_set_dev_dir(const char *dev_dir)
 	slash = dev_dir[len-1] == '/' ? "" : "/";
 
 	if (snprintf(_dm_dir, sizeof _dm_dir, "%s%s%s", dev_dir, slash, DM_DIR)
-	    >= sizeof _dm_dir) {
+	    >= (int) sizeof(_dm_dir)) {
 		log_debug("Invalid dev_dir value, %s: name too long.", dev_dir);
 		return 0;
 	}
-- 
1.7.4.1

[PATCH 6/6] const fixes

[Zdenek Kabelac]

When recent update dm_report_field_string() API call to accept
completely const object - we no longer need loose const here
and keep it forwarding.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 tools/dmsetup.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/dmsetup.c b/tools/dmsetup.c
index 5455369..362d7e7 100644
--- a/tools/dmsetup.c
+++ b/tools/dmsetup.c
@@ -2456,7 +2456,7 @@ static int _dm_subsystem_disp(struct dm_report *rh,
 			       struct dm_report_field *field, const void *data,
 			       void *private __attribute__((unused)))
 {
-	return dm_report_field_string(rh, field, (const char **) data);
+	return dm_report_field_string(rh, field, (const char *const *) data);
 }
 
 static int _dm_vg_name_disp(struct dm_report *rh,
@@ -2465,7 +2465,7 @@ static int _dm_vg_name_disp(struct dm_report *rh,
 			     void *private __attribute__((unused)))
 {
 
-	return dm_report_field_string(rh, field, (const char **) data);
+	return dm_report_field_string(rh, field, (const char *const *) data);
 }
 
 static int _dm_lv_name_disp(struct dm_report *rh,
@@ -2474,7 +2474,7 @@ static int _dm_lv_name_disp(struct dm_report *rh,
 			     void *private __attribute__((unused)))
 
 {
-	return dm_report_field_string(rh, field, (const char **) data);
+	return dm_report_field_string(rh, field, (const char *const *) data);
 }
 
 static int _dm_lv_layer_name_disp(struct dm_report *rh,
@@ -2483,7 +2483,7 @@ static int _dm_lv_layer_name_disp(struct dm_report *rh,
 				   void *private __attribute__((unused)))
 
 {
-	return dm_report_field_string(rh, field, (const char **) data);
+	return dm_report_field_string(rh, field, (const char *const *) data);
 }
 
 static void *_task_get_obj(void *obj)
-- 
1.7.4.1