diff mbox series

[nft,4/5] datatype: extend set_datatype_alloc() to change size

Message ID 20230927200143.3798124-5-thaller@redhat.com
State New
Headers show
Series more various cleanups related to struct datatype | expand

Commit Message

Thomas Haller Sept. 27, 2023, 7:57 p.m. UTC 
All remaining users of datatype_clone() only use this function to set
the byteorder and the size. Extend set_datatype_alloc() to not only
change the byteorder but also the size. With that, it can be used
instead of clone.

The benefit is that set_datatype_alloc() takes care to re-use the same
instance, if the values end up being the same.

Another benefit is to expose and make clear the oddity related to the
"for_any_integer" argument. The argument exists to preserve the previous
behavior.  However, it's not clear why some places would only want to
change the values for &integer_type and some check for orig_dtype->type
== TYPE_INTEGER. This is possibly a bug, especially because it means
once you clone an &integer_type, you cannot change it's byteorder/size
again.  So &integer_type is very special here, not only based on the
TYPE_INTEGER.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 include/datatype.h |  5 ++++-
 src/datatype.c     | 21 ++++++++++++++++-----
 src/evaluate.c     | 26 ++++++++++++--------------
 src/netlink.c      |  4 ++--
 src/payload.c      |  7 +++----
 5 files changed, 37 insertions(+), 26 deletions(-)
diff mbox series

Patch

diff --git a/include/datatype.h b/include/datatype.h
index 465ade290652..f01e15b6ff3e 100644
--- a/include/datatype.h
+++ b/include/datatype.h
@@ -301,7 +301,10 @@  concat_subtype_lookup(uint32_t type, unsigned int n)
 }
 
 extern const struct datatype *
-set_datatype_alloc(const struct datatype *orig_dtype, enum byteorder byteorder);
+set_datatype_alloc(const struct datatype *orig_dtype,
+		   bool for_any_integer,
+		   enum byteorder byteorder,
+		   unsigned int size);
 
 extern void time_print(uint64_t msec, struct output_ctx *octx);
 extern struct error_record *time_parse(const struct location *loc,
diff --git a/src/datatype.c b/src/datatype.c
index 6a35c6a76028..f9570603467a 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1313,21 +1313,32 @@  const struct datatype *concat_type_alloc(uint32_t type)
 }
 
 const struct datatype *set_datatype_alloc(const struct datatype *orig_dtype,
-					  enum byteorder byteorder)
+					  bool for_any_integer,
+					  enum byteorder byteorder,
+					  unsigned int size)
 {
 	struct datatype *dtype;
 
-	/* Restrict dynamic datatype allocation to generic integer datatype. */
-	if (orig_dtype != &integer_type)
-		return datatype_get(orig_dtype);
+	if (for_any_integer) {
+		if (orig_dtype->type != TYPE_INTEGER) {
+			/* Restrict changing byteorder/size to any integer datatype. */
+			return datatype_get(orig_dtype);
+		}
+	} else {
+		if (orig_dtype != &integer_type) {
+			/* Restrict changing byteorder/size to the generic integer datatype. */
+			return datatype_get(orig_dtype);
+		}
+	}
 
-	if (orig_dtype->byteorder == byteorder) {
+	if (orig_dtype->byteorder == byteorder && orig_dtype->size == size) {
 		/* The (immutable) type instance is already as requested. */
 		return datatype_get(orig_dtype);
 	}
 
 	dtype = datatype_clone(orig_dtype);
 	dtype->byteorder = byteorder;
+	dtype->size = size;
 	return dtype;
 }
 
diff --git a/src/evaluate.c b/src/evaluate.c
index e84895bf1610..a118aa6a7209 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -89,7 +89,8 @@  static struct expr *implicit_set_declaration(struct eval_ctx *ctx,
 	if (set_is_datamap(expr->set_flags)) {
 		const struct datatype *dtype;
 
-		dtype = set_datatype_alloc(key->dtype, key->byteorder);
+		dtype = set_datatype_alloc(key->dtype, false, key->byteorder,
+					   key->dtype->size);
 		__datatype_set(key, dtype);
 	}
 
@@ -1508,13 +1509,12 @@  static int expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
 			return -1;
 		flags &= i->flags;
 
-		if (!key && i->dtype->type == TYPE_INTEGER) {
-			struct datatype *clone;
+		if (!key) {
+			const struct datatype *dtype;
 
-			clone = datatype_clone(i->dtype);
-			clone->size = i->len;
-			clone->byteorder = i->byteorder;
-			__datatype_set(i, clone);
+			dtype = set_datatype_alloc(i->dtype, true,
+						   i->byteorder, i->len);
+			__datatype_set(i, dtype);
 		}
 
 		if (dtype == NULL && i->dtype->size == 0)
@@ -1933,7 +1933,7 @@  static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr)
 		} else {
 			const struct datatype *dtype;
 
-			dtype = set_datatype_alloc(ectx.dtype, ectx.byteorder);
+			dtype = set_datatype_alloc(ectx.dtype, false, ectx.byteorder, ectx.dtype->size);
 			data = constant_expr_alloc(&netlink_location, dtype,
 						   dtype->byteorder, ectx.len, NULL);
 			datatype_free(dtype);
@@ -4553,13 +4553,11 @@  static int set_expr_evaluate_concat(struct eval_ctx *ctx, struct expr **expr)
 			return expr_error(ctx->msgs, i,
 					  "specify either ip or ip6 for address matching");
 
-		if (i->etype == EXPR_PAYLOAD &&
-		    i->dtype->type == TYPE_INTEGER) {
-			struct datatype *dtype;
+		if (i->etype == EXPR_PAYLOAD) {
+			const struct datatype *dtype;
 
-			dtype = datatype_clone(i->dtype);
-			dtype->size = i->len;
-			dtype->byteorder = i->byteorder;
+			dtype = set_datatype_alloc(i->dtype, true,
+						   i->byteorder, i->len);
 			__datatype_set(i, dtype);
 		}
 
diff --git a/src/netlink.c b/src/netlink.c
index 120a8ba9ceb1..9fe870885e2e 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1034,7 +1034,7 @@  struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 	if (datatype) {
 		uint32_t dlen;
 
-		dtype2 = set_datatype_alloc(datatype, databyteorder);
+		dtype2 = set_datatype_alloc(datatype, false, databyteorder, datatype->size);
 		klen = nftnl_set_get_u32(nls, NFTNL_SET_DATA_LEN) * BITS_PER_BYTE;
 
 		dlen = data_interval ?  klen / 2 : klen;
@@ -1058,7 +1058,7 @@  struct set *netlink_delinearize_set(struct netlink_ctx *ctx,
 			set->data->flags |= EXPR_F_INTERVAL;
 	}
 
-	dtype = set_datatype_alloc(keytype, keybyteorder);
+	dtype = set_datatype_alloc(keytype, false, keybyteorder, keytype->size);
 	klen = nftnl_set_get_u32(nls, NFTNL_SET_KEY_LEN) * BITS_PER_BYTE;
 
 	if (set_udata_key_valid(typeof_expr_key, klen)) {
diff --git a/src/payload.c b/src/payload.c
index 89bb38eb0099..55e075dab033 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -243,15 +243,14 @@  static struct expr *payload_expr_parse_udata(const struct nftnl_udata *attr)
 		expr->len = len;
 
 	if (is_raw) {
-		struct datatype *dtype;
+		const struct datatype *dtype;
 
 		expr->payload.base = base;
 		expr->payload.offset = offset;
 		expr->payload.is_raw = true;
 		expr->len = len;
-		dtype = datatype_clone(&xinteger_type);
-		dtype->size = len;
-		dtype->byteorder = BYTEORDER_BIG_ENDIAN;
+		dtype = set_datatype_alloc(&xinteger_type, true,
+					   BYTEORDER_BIG_ENDIAN, len);
 		__datatype_set(expr, dtype);
 	}