[Cluster-devel] [PATCH dlm-tool 0/4] dlm_controld: gcc-10 compile warning fixes

[Cluster-devel] [PATCH dlm-tool 0/4] dlm_controld: gcc-10 compile warning fixes

[Alexander Aring]

Hi,

this patch series contains fixes to avoid compile warnings for gcc
version 10.

There might be some points for code sharing which I ignored right now
and leave the code as it is.

Also I added a patch to more log functionality in cases that a key value
config entry isn't successful parsed.

- Alex

Alexander Aring (4):
  dlm_controld: fix -Wstringop-truncation warnings
  dlm_controld: fix may be used uninitialized
  dlm_controld: fix may be used uninitialized
  dlm_controld: get notice about failed config parse

 dlm_controld/config.c       | 14 ++++++++++----
 dlm_controld/cpg.c          |  3 ++-
 dlm_controld/daemon_cpg.c   |  4 ++--
 dlm_controld/fence_config.c | 20 +++++++++++++++-----
 dlm_controld/lib.c          | 15 +++++++++++++--
 dlm_controld/main.c         | 13 ++++++++++++-
 6 files changed, 54 insertions(+), 15 deletions(-)

-- 
2.26.2

[Cluster-devel] [PATCH dlm-tool 1/4] dlm_controld: fix -Wstringop-truncation warnings

[Alexander Aring]

This patch fixes in dlm_controld all -Wstringop-truncation warnings.
There exists two different cases inside the code:

1. string buffer without null termination:

Code work as expected in this case a pragma is introduced to ignore the
warning.

2. string buffer with null termination:

The function strncpy() will not leave the destination buffer with a null
termination symbol if the buffer doesn't fit. That's why gcc above 8.0
print warnings. Obviously there are some memset() to zero the buffer and
doing a strncpy() minus one of the buffer length afterwards which seems
fine. The fact that gcc still complains and knowing other discussions
about memset() I believe that there might be reasons why gcc doesn't
stop to complain about such code or gcc isn't able to detect it.

However this patch will guarantee that the destination buffer is always
null terminated and the full destination buffer size is used now.
---
 dlm_controld/cpg.c          |  3 ++-
 dlm_controld/fence_config.c | 20 +++++++++++++++-----
 dlm_controld/lib.c          | 15 +++++++++++++--
 dlm_controld/main.c         | 13 ++++++++++++-
 4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/dlm_controld/cpg.c b/dlm_controld/cpg.c
index 5b5c52fc..f3365ee4 100644
--- a/dlm_controld/cpg.c
+++ b/dlm_controld/cpg.c
@@ -1867,7 +1867,8 @@ int set_lockspace_info(struct lockspace *ls, struct dlmc_lockspace *lockspace)
 {
 	struct change *cg, *last = NULL;
 
-	strncpy(lockspace->name, ls->name, DLM_LOCKSPACE_LEN);
+	strncpy(lockspace->name, ls->name, DLM_LOCKSPACE_LEN + 1);
+	lockspace->name[DLM_LOCKSPACE_LEN] = '\0';
 	lockspace->global_id = ls->global_id;
 
 	if (ls->joining)
diff --git a/dlm_controld/fence_config.c b/dlm_controld/fence_config.c
index 5d8d7dc1..08996ac0 100644
--- a/dlm_controld/fence_config.c
+++ b/dlm_controld/fence_config.c
@@ -180,11 +180,21 @@ static int read_config_section(unsigned int nodeid, FILE *file, char *dev_line,
 		memset(dev, 0, sizeof(struct fence_device));
 		memset(con, 0, sizeof(struct fence_connect));
 
-		strncpy(dev->name, dev_name, FENCE_CONFIG_NAME_MAX-1);
-		strncpy(dev->agent, agent, FENCE_CONFIG_NAME_MAX-1);
-		strncpy(dev->args, dev_args, FENCE_CONFIG_ARGS_MAX-1);
-		strncpy(con->name, con_name, FENCE_CONFIG_NAME_MAX-1);
-		strncpy(con->args, con_args, FENCE_CONFIG_ARGS_MAX-1);
+		strncpy(dev->name, dev_name, FENCE_CONFIG_NAME_MAX);
+		dev->name[FENCE_CONFIG_NAME_MAX - 1] = '\0';
+
+		strncpy(dev->agent, agent, FENCE_CONFIG_NAME_MAX);
+		dev->agent[FENCE_CONFIG_NAME_MAX - 1] = '\0';
+
+		strncpy(dev->args, dev_args, FENCE_CONFIG_ARGS_MAX);
+		dev->args[FENCE_CONFIG_ARGS_MAX - 1] = '\0';
+
+		strncpy(con->name, con_name, FENCE_CONFIG_NAME_MAX);
+		con->name[FENCE_CONFIG_NAME_MAX - 1] = '\0';
+
+		strncpy(con->args, con_args, FENCE_CONFIG_ARGS_MAX);
+		con->args[FENCE_CONFIG_ARGS_MAX - 1] = '\0';
+
 		dev->unfence = unfence;
 
 		*dev_out = dev;
diff --git a/dlm_controld/lib.c b/dlm_controld/lib.c
index b6ea3a30..53c11cf9 100644
--- a/dlm_controld/lib.c
+++ b/dlm_controld/lib.c
@@ -81,6 +81,17 @@ static int do_connect(const char *sock_path)
 	return fd;
 }
 
+static inline void init_header_name(struct dlmc_header *h,
+				    const char *name, size_t len)
+{
+#pragma GCC diagnostic push
+#if __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
+	strncpy(h->name, name, len);
+#pragma GCC diagnostic pop
+}
+
 static void init_header(struct dlmc_header *h, int cmd, char *name,
 			int extra_len)
 {
@@ -92,7 +103,7 @@ static void init_header(struct dlmc_header *h, int cmd, char *name,
 	h->command = cmd;
 
 	if (name)
-		strncpy(h->name, name, DLM_LOCKSPACE_LEN);
+		init_header_name(h, name, DLM_LOCKSPACE_LEN);
 }
 
 static char copy_buf[DLMC_DUMP_SIZE];
@@ -881,7 +892,7 @@ int dlmc_run_check(char *run_uuid, int len, int wait_sec, uint32_t flags,
 
 	init_header(&h, DLMC_CMD_RUN_CHECK, NULL, 0);
 	h.flags = flags;
-	strncpy(h.name, run_uuid, DLMC_RUN_UUID_LEN);
+	init_header_name(&h, run_uuid, DLMC_RUN_UUID_LEN);
 
 	memset(&rh, 0, sizeof(rh));
 
diff --git a/dlm_controld/main.c b/dlm_controld/main.c
index 8023f4b0..645bd26b 100644
--- a/dlm_controld/main.c
+++ b/dlm_controld/main.c
@@ -788,6 +788,17 @@ static int setup_uevent(void)
 	return s;
 }
 
+static inline void init_header_name(struct dlmc_header *h,
+				    const char *name, size_t len)
+{
+#pragma GCC diagnostic push
+#if __GNUC__ >= 8
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
+	strncpy(h->name, name, len);
+#pragma GCC diagnostic pop
+}
+
 static void init_header(struct dlmc_header *h, int cmd, char *name, int result,
 			int extra_len)
 {
@@ -800,7 +811,7 @@ static void init_header(struct dlmc_header *h, int cmd, char *name, int result,
 	h->data = result;
 
 	if (name)
-		strncpy(h->name, name, DLM_LOCKSPACE_LEN);
+		init_header_name(h, name, DLM_LOCKSPACE_LEN);
 }
 
 static char copy_buf[LOG_DUMP_SIZE];
-- 
2.26.2

[Cluster-devel] [PATCH dlm-tool 2/4] dlm_controld: fix may be used uninitialized

[Alexander Aring]

This patch fixes the following compiler warning:

daemon_cpg.c:2158:12: warning: ?run? may be used uninitialized in this function [-Wmaybe-uninitialized]
 2158 |   run->info.reply_count++;

It's fixed by just init the used variable to NULL and use it only if
it's not NULL.
---
 dlm_controld/daemon_cpg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlm_controld/daemon_cpg.c b/dlm_controld/daemon_cpg.c
index 99aca8db..392e0ff6 100644
--- a/dlm_controld/daemon_cpg.c
+++ b/dlm_controld/daemon_cpg.c
@@ -2115,7 +2115,7 @@ int receive_run_reply(struct dlm_header *hd, int len)
 int receive_run_request(struct dlm_header *hd, int len)
 {
 	struct run_request *req = (struct run_request *)hd;
-	struct run *run;
+	struct run *run = NULL;
 
 	run_request_in(req);
 
@@ -2153,7 +2153,7 @@ int receive_run_request(struct dlm_header *hd, int len)
 		return 0;
 	}
 
-	if (!opt(enable_helper_ind)) {
+	if (!opt(enable_helper_ind) && run) {
 		log_debug("receive_run_request %s helper not enabled", req->uuid);
 		run->info.reply_count++;
 		run->info.need_replies--;
-- 
2.26.2

[Cluster-devel] [PATCH dlm-tool 3/4] dlm_controld: fix may be used uninitialized

[Alexander Aring]

This patch fixes the following compiler warning:

config.c:269:26: warning: ?val? may be used uninitialized in this function [-Wmaybe-uninitialized]
  269 |    o->file_int = val ? 1 : 0;

The fix just init's val to 0 to have a fallback if parsing fails.
---
 dlm_controld/config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlm_controld/config.c b/dlm_controld/config.c
index ec269168..323f91e9 100644
--- a/dlm_controld/config.c
+++ b/dlm_controld/config.c
@@ -189,7 +189,7 @@ void set_opt_file(int update)
 	FILE *file;
 	char line[MAX_LINE];
 	char str[MAX_LINE];
-	int i, val;
+	int i, val = 0;
 
 	if (!path_exists(CONF_FILE_PATH))
 		return;
-- 
2.26.2

[Cluster-devel] [PATCH dlm-tool 4/4] dlm_controld: get notice about failed config parse

[Alexander Aring]

This patch adds more log_error() functionality in cases if parsing of
key value pairs fails so the user get notice about it.
---
 dlm_controld/config.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/dlm_controld/config.c b/dlm_controld/config.c
index 323f91e9..947480da 100644
--- a/dlm_controld/config.c
+++ b/dlm_controld/config.c
@@ -150,8 +150,10 @@ static void get_val_int(char *line, int *val_out)
 	int rv;
 
 	rv = sscanf(line, "%[^=]=%s", key, val);
-	if (rv != 2)
+	if (rv != 2) {
+		log_error("Failed to parse config line %s", line);
 		return;
+	}
 
 	*val_out = atoi(val);
 }
@@ -163,8 +165,10 @@ static void get_val_uint(char *line, unsigned int *val_out)
 	int rv;
 
 	rv = sscanf(line, "%[^=]=%s", key, val);
-	if (rv != 2)
+	if (rv != 2) {
+		log_error("Failed to parse config line %s", line);
 		return;
+	}
 
 	*val_out = strtoul(val, NULL, 0);
 }
@@ -176,8 +180,10 @@ static void get_val_str(char *line, char *val_out)
 	int rv;
 
 	rv = sscanf(line, "%[^=]=%s", key, val);
-	if (rv != 2)
+	if (rv != 2) {
+		log_error("Failed to parse config line %s", line);
 		return;
+	}
 
 	strcpy(val_out, val);
 }
-- 
2.26.2