[PATCH nft 5/8] src: rework SNPRINTF_BUFFER_SIZE() and avoid "-Wunused-but-set-variable"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



SNPRINTF_BUFFER_SIZE() causes a warning with clang.

    evaluate.c:4134:9: error: variable 'size' set but not used [-Werror,-Wunused-but-set-variable]
            size_t size = 0;
                   ^

    meta.c:1006:9: error: variable 'size' set but not used [-Werror,-Wunused-but-set-variable]
            size_t size;
                   ^

Fix that, and rework SNPRINTF_BUFFER_SIZE().

- before and now, the macro asserts against truncation. Remove error
  handling related to truncation in the callers.

- wrap the macro in "do { ... } while(0)" to make it more
  function-like.

- evaluate macro arguments exactly once, to make it more function-like.

- take pointers to the arguments that are being modified.

- use assert() instead of abort().

- use size_t type for arguments related to the buffer size.

- drop "size". It was unused, and, unless the string was truncated,
  it was identical to "offset".

- "offset" previously was incremented before checking for truncation.
  So it would point somewhere past the buffer. This behavior is not
  useful, because we assert against truncation. Now, in case of
  truncation, "len" will be zero and "offset" will be the original "len"
  (that is, point after the buffer and one byte after the terminating NUL).

Signed-off-by: Thomas Haller <thaller@xxxxxxxxxx>
---
 include/utils.h | 32 +++++++++++++++++++++++---------
 src/evaluate.c  | 11 +++++------
 src/meta.c      | 10 +++++-----
 3 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index cee1e5c1e8ae..873147fb54ec 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -72,15 +72,29 @@
 #define max(_x, _y) ({				\
 	_x > _y ? _x : _y; })
 
-#define SNPRINTF_BUFFER_SIZE(ret, size, len, offset)	\
-	if (ret < 0)					\
-		abort();				\
-	offset += ret;					\
-	assert(ret < len);				\
-	if (ret > len)					\
-		ret = len;				\
-	size += ret;					\
-	len -= ret;
+#define SNPRINTF_BUFFER_SIZE(ret, len, offset)			\
+	do { \
+		const int _ret = (ret);				\
+		size_t *const _len = (len);			\
+		size_t *const _offset = (offset);		\
+		size_t _ret2;					\
+								\
+		assert(_ret >= 0);				\
+								\
+		if ((size_t) _ret >= *_len) {			\
+			/* Fail an assertion on truncation.
+			 *
+			 * Anyway, we would set "len" to zero and "offset" one
+			 * after the buffer size (past the terminating NUL
+			 * byte). */				\
+			assert((size_t) _ret < *_len);		\
+			_ret2 = *_len;				\
+		} else						\
+			_ret2 = (size_t) _ret;			\
+								\
+		*_offset += _ret2;				\
+		*_len -= _ret2;					\
+	} while (0)
 
 #define MSEC_PER_SEC	1000L
 
diff --git a/src/evaluate.c b/src/evaluate.c
index 1ae2ef0de10c..f8cd7b7afda3 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4129,14 +4129,16 @@ static int stmt_evaluate_queue(struct eval_ctx *ctx, struct stmt *stmt)
 static int stmt_evaluate_log_prefix(struct eval_ctx *ctx, struct stmt *stmt)
 {
 	char prefix[NF_LOG_PREFIXLEN] = {}, tmp[NF_LOG_PREFIXLEN] = {};
-	int len = sizeof(prefix), offset = 0, ret;
+	size_t len = sizeof(prefix);
+	size_t offset = 0;
 	struct expr *expr;
-	size_t size = 0;
 
 	if (stmt->log.prefix->etype != EXPR_LIST)
 		return 0;
 
 	list_for_each_entry(expr, &stmt->log.prefix->expressions, list) {
+		int ret;
+
 		switch (expr->etype) {
 		case EXPR_VALUE:
 			expr_to_string(expr, tmp);
@@ -4150,12 +4152,9 @@ static int stmt_evaluate_log_prefix(struct eval_ctx *ctx, struct stmt *stmt)
 			BUG("unknown expression type %s\n", expr_name(expr));
 			break;
 		}
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
+		SNPRINTF_BUFFER_SIZE(ret, &len, &offset);
 	}
 
-	if (len == NF_LOG_PREFIXLEN)
-		return stmt_error(ctx, stmt, "log prefix is too long");
-
 	expr = constant_expr_alloc(&stmt->log.prefix->location, &string_type,
 				   BYTEORDER_HOST_ENDIAN,
 				   strlen(prefix) * BITS_PER_BYTE, prefix);
diff --git a/src/meta.c b/src/meta.c
index 4f383269d032..6dada9719e5b 100644
--- a/src/meta.c
+++ b/src/meta.c
@@ -999,11 +999,11 @@ struct error_record *meta_key_parse(const struct location *loc,
                                     const char *str,
                                     unsigned int *value)
 {
-	int ret, len, offset = 0;
 	const char *sep = "";
+	size_t offset = 0;
 	unsigned int i;
 	char buf[1024];
-	size_t size;
+	size_t len;
 
 	for (i = 0; i < array_size(meta_templates); i++) {
 		if (!meta_templates[i].token || strcmp(meta_templates[i].token, str))
@@ -1026,9 +1026,10 @@ struct error_record *meta_key_parse(const struct location *loc,
 	}
 
 	len = (int)sizeof(buf);
-	size = sizeof(buf);
 
 	for (i = 0; i < array_size(meta_templates); i++) {
+		int ret;
+
 		if (!meta_templates[i].token)
 			continue;
 
@@ -1036,8 +1037,7 @@ struct error_record *meta_key_parse(const struct location *loc,
 			sep = ", ";
 
 		ret = snprintf(buf+offset, len, "%s%s", sep, meta_templates[i].token);
-		SNPRINTF_BUFFER_SIZE(ret, size, len, offset);
-		assert(offset < (int)sizeof(buf));
+		SNPRINTF_BUFFER_SIZE(ret, &len, &offset);
 	}
 
 	return error(loc, "syntax error, unexpected %s, known keys are %s", str, buf);
-- 
2.41.0




[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux