* [Cluster-devel] [PATCH dlm-tool 1/4] dlm_controld: fix -Wstringop-truncation warnings
2020-07-14 18:01 [Cluster-devel] [PATCH dlm-tool 0/4] dlm_controld: gcc-10 compile warning fixes Alexander Aring
@ 2020-07-14 18:01 ` Alexander Aring
2020-07-14 18:01 ` [Cluster-devel] [PATCH dlm-tool 2/4] dlm_controld: fix may be used uninitialized Alexander Aring
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Alexander Aring @ 2020-07-14 18:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH dlm-tool 4/4] dlm_controld: get notice about failed config parse
2020-07-14 18:01 [Cluster-devel] [PATCH dlm-tool 0/4] dlm_controld: gcc-10 compile warning fixes Alexander Aring
` (2 preceding siblings ...)
2020-07-14 18:01 ` [Cluster-devel] [PATCH dlm-tool 3/4] " Alexander Aring
@ 2020-07-14 18:01 ` Alexander Aring
3 siblings, 0 replies; 5+ messages in thread
From: Alexander Aring @ 2020-07-14 18:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
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
^ permalink raw reply related [flat|nested] 5+ messages in thread