src: rework SNPRINTF_BUFFER_SIZE() and handle truncation

Before, the macro asserts against truncation. This is despite the
callers still checked for truncation and tried to handle it. Probably
for good reason. With stmt_evaluate_log_prefix() it's not clear that the
code ensures that truncation cannot happen, so we must not assert
against it, but handle it.

Also,

- 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 mostly redundant to "offset". We can know
  everything we want based on "len" and "offset" alone.

- "offset" previously was incremented before checking for truncation.
  So it would point somewhere past the buffer. This behavior does not
  seem useful. Instead, on truncation "len" will be zero (as before) and
  "offset" will point one past the buffer (one past the terminating
  NUL).

Thereby, also fix a warning from 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;
                   ^

Signed-off-by: Thomas Haller <thaller@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/utils.h b/include/utils.h
index cee1e5c1e8aed00ce9392e925126beb629734829..efc8dec013a1092a3a3e5686ec48c39341973a61 100644 (file)
--- a/include/utils.h
+++ b/include/utils.h
@@ -72,15 +72,32 @@
 #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)                 \
+       ({ \
+               const int _ret = (ret);                         \
+               size_t *const _len = (len);                     \
+               size_t *const _offset = (offset);               \
+               bool _not_truncated = true;                     \
+               size_t _ret2;                                   \
+                                                               \
+               assert(_ret >= 0);                              \
+                                                               \
+               if ((size_t) _ret >= *_len) {                   \
+                       /* Truncated.
+                        *
+                        * We will leave "len" at zero and increment
+                        * "offset" to point one byte after the buffer
+                        * (after the terminating NUL byte). */ \
+                       _ret2 = *_len;                          \
+                       _not_truncated = false;                 \
+               } else                                          \
+                       _ret2 = (size_t) _ret;                  \
+                                                               \
+               *_offset += _ret2;                              \
+               *_len -= _ret2;                                 \
+                                                               \
+               _not_truncated;                                 \
+       })
 
 #define MSEC_PER_SEC   1000L
 

diff --git a/src/evaluate.c b/src/evaluate.c
index 4c02a9cd821c9fb3d55f88e33be5773c6d7650bc..83c4b045818031e775561307b860f75c2709277e 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4126,14 +4126,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);
@@ -4147,7 +4149,7 @@ 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 == 0)

diff --git a/src/meta.c b/src/meta.c
index 4f383269d032f3cc25e5683aec08b10334847cac..ea00f2396b994f1a0b2442d7b3342fcf5b4ed961 100644 (file)
--- 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,8 @@ 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);
+               assert(len > 0);
        }
 
        return error(loc, "syntax error, unexpected %s, known keys are %s", str, buf);