[PATCH conntrack-tools] conntrack: fix compiler warnings

[PATCH conntrack-tools] conntrack: fix compiler warnings

[Florian Westphal]

.... those do not indicate bugs, but they are distracting.

'exp_filter_add' at filter.c:513:2:
__builtin_strncpy specified bound 16 equals destination size [-Wstringop-truncation]
read_config_yy.y:1625: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=]
 1625 |         snprintf(policy->name, CTD_HELPER_NAME_LEN, "%s", $2);
read_config_yy.y:1399: warning: '__builtin_snprintf' output may be ...
 1399 |         snprintf(conf.stats.logfile, FILENAME_MAXLEN, "%s", $2);
read_config_yy.y:707: warning: '__builtin_snprintf' output may be ...
  707 |         snprintf(conf.local.path, UNIX_PATH_MAX, "%s", $2);
read_config_yy.y:179: warning: '__builtin_snprintf' output may be ...
  179 |         snprintf(conf.lockfile, FILENAME_MAXLEN, "%s", $2);
read_config_yy.y:124: warning: '__builtin_snprintf' output may be ...
  124 |         snprintf(conf.logfile, FILENAME_MAXLEN, "%s", $2);

... its because the _MAXLEN constants are one less than the output
buffer size, i.e. could use either .._MAXLEN + 1 or sizeof, this uses
sizeof().

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/filter.c         |  2 +-
 src/read_config_yy.y | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/filter.c b/src/filter.c
index 65771025308f..41f9bd793f03 100644
--- a/src/filter.c
+++ b/src/filter.c
@@ -510,7 +510,7 @@ int exp_filter_add(struct exp_filter *f, const char *helper_name)
 	if (item == NULL)
 		return -1;
 
-	strncpy(item->helper_name, helper_name, NFCT_HELPER_NAME_MAX);
+	strncpy(item->helper_name, helper_name, NFCT_HELPER_NAME_MAX - 1);
 	list_add(&item->head, &f->list);
 	return 0;
 }
diff --git a/src/read_config_yy.y b/src/read_config_yy.y
index 95845a19e768..070b349c5949 100644
--- a/src/read_config_yy.y
+++ b/src/read_config_yy.y
@@ -121,7 +121,7 @@ logfile_path : T_LOG T_PATH_VAL
 		     FILENAME_MAXLEN);
 		exit(EXIT_FAILURE);
 	}
-	snprintf(conf.logfile, FILENAME_MAXLEN, "%s", $2);
+	snprintf(conf.logfile, sizeof(conf.logfile), "%s", $2);
 	free($2);
 };
 
@@ -176,7 +176,7 @@ lock : T_LOCK T_PATH_VAL
 		     FILENAME_MAXLEN);
 		exit(EXIT_FAILURE);
 	}
-	snprintf(conf.lockfile, FILENAME_MAXLEN, "%s", $2);
+	snprintf(conf.lockfile, sizeof(conf.lockfile), "%s", $2);
 	free($2);
 };
 
@@ -704,7 +704,7 @@ unix_option : T_PATH T_PATH_VAL
 		     UNIX_PATH_MAX);
 		exit(EXIT_FAILURE);
 	}
-	snprintf(conf.local.path, UNIX_PATH_MAX, "%s", $2);
+	snprintf(conf.local.path, sizeof(conf.local.path), "%s", $2);
 	free($2);
 };
 
@@ -1396,7 +1396,7 @@ stat_logfile_path : T_LOG T_PATH_VAL
 		     FILENAME_MAXLEN);
 		exit(EXIT_FAILURE);
 	}
-	snprintf(conf.stats.logfile, FILENAME_MAXLEN, "%s", $2);
+	snprintf(conf.stats.logfile, sizeof(conf.stats.logfile), "%s", $2);
 	free($2);
 };
 
@@ -1622,7 +1622,7 @@ helper_type: T_HELPER_POLICY T_STRING '{' helper_policy_list '}'
 	}
 
 	policy = (struct ctd_helper_policy *) &e->data;
-	snprintf(policy->name, CTD_HELPER_NAME_LEN, "%s", $2);
+	snprintf(policy->name, sizeof(policy->name), "%s", $2);
 	free($2);
 	/* Now object is complete. */
 	e->type = SYMBOL_HELPER_POLICY_EXPECT_ROOT;
-- 
2.34.1

Re: [PATCH conntrack-tools] conntrack: fix compiler warnings

[Pablo Neira Ayuso]

On Mon, Jan 17, 2022 at 04:42:52PM +0100, Florian Westphal wrote:
> .... those do not indicate bugs, but they are distracting.
> 
> 'exp_filter_add' at filter.c:513:2:
> __builtin_strncpy specified bound 16 equals destination size [-Wstringop-truncation]
> read_config_yy.y:1625: warning: '__builtin_snprintf' output may be truncated before the last format character [-Wformat-truncation=]
>  1625 |         snprintf(policy->name, CTD_HELPER_NAME_LEN, "%s", $2);
> read_config_yy.y:1399: warning: '__builtin_snprintf' output may be ...
>  1399 |         snprintf(conf.stats.logfile, FILENAME_MAXLEN, "%s", $2);
> read_config_yy.y:707: warning: '__builtin_snprintf' output may be ...
>   707 |         snprintf(conf.local.path, UNIX_PATH_MAX, "%s", $2);
> read_config_yy.y:179: warning: '__builtin_snprintf' output may be ...
>   179 |         snprintf(conf.lockfile, FILENAME_MAXLEN, "%s", $2);
> read_config_yy.y:124: warning: '__builtin_snprintf' output may be ...
>   124 |         snprintf(conf.logfile, FILENAME_MAXLEN, "%s", $2);
> 
> ... its because the _MAXLEN constants are one less than the output
> buffer size, i.e. could use either .._MAXLEN + 1 or sizeof, this uses
> sizeof().

LGTM, thanks