Re: [PATCH net-next v8 03/10] lib: packing: add pack_fields() and unpack_fields()

That incremental improvement is below, if you'd be interested in including it
(the auto-generated code is not part of the diff):

diff --git a/include/linux/packing.h b/include/linux/packing.h
index c4fc76ae64a5..1c89a5129b06 100644
--- a/include/linux/packing.h
+++ b/include/linux/packing.h

@@ -36,22 +36,38 @@ struct packed_field_m {
 	sizeof_field(struct_name, struct_field), \
 }
 
-#define CHECK_PACKED_FIELD(field) ({ \
-	typeof(field) __f = (field); \
-	BUILD_BUG_ON(__f.startbit < __f.endbit); \
-	BUILD_BUG_ON(__f.startbit - __f.endbit >= BITS_PER_BYTE * __f.size); \
-	BUILD_BUG_ON(__f.size != 1 && __f.size != 2 && \
-		     __f.size != 4 && __f.size != 8); \
+#define CHECK_PACKED_FIELD_OVERLAP(fields, index1, index2) ({ \
+	typeof(&(fields)[0]) __f = (fields); \
+	typeof(__f[0]) _f1 = __f[index1]; typeof(__f[0]) _f2 = __f[index2]; \
+	const bool _ascending = __f[0].startbit < __f[1].startbit; \
+	BUILD_BUG_ON_MSG(_ascending && _f1.startbit >= _f2.startbit, \
+			 __stringify(fields) " field " __stringify(index2) \
+			 " breaks ascending order"); \
+	BUILD_BUG_ON_MSG(!_ascending && _f1.startbit <= _f2.startbit, \
+			 __stringify(fields) " field " __stringify(index2) \
+			 " breaks descending order"); \
+	BUILD_BUG_ON_MSG(max(_f1.endbit, _f2.endbit) <= \
+			 min(_f1.startbit, _f2.startbit), \
+			 __stringify(fields) " field " __stringify(index2) \
+			 " overlaps with previous field"); \
 })
 
-
-#define CHECK_PACKED_FIELD_OVERLAP(ascending, field1, field2) ({ \
-	typeof(field1) _f1 = (field1); typeof(field2) _f2 = (field2); \
-	const bool _a = (ascending); \
-	BUILD_BUG_ON(_a && _f1.startbit >= _f2.startbit); \
-	BUILD_BUG_ON(!_a && _f1.startbit <= _f2.startbit); \
-	BUILD_BUG_ON(max(_f1.endbit, _f2.endbit) <= \
-		     min(_f1.startbit, _f2.startbit)); \
+#define CHECK_PACKED_FIELD(fields, index) ({ \
+	typeof(&(fields)[0]) _f = (fields); \
+	typeof(_f[0]) __f = _f[index]; \
+	BUILD_BUG_ON_MSG(__f.startbit < __f.endbit, \
+			 __stringify(fields) " field " __stringify(index) \
+			 " start bit must not be smaller than end bit"); \
+	BUILD_BUG_ON_MSG(__f.size != 1 && __f.size != 2 && \
+			 __f.size != 4 && __f.size != 8, \
+			 __stringify(fields) " field " __stringify(index) \
+			" has unsupported unpacked storage size"); \
+	BUILD_BUG_ON_MSG(__f.startbit - __f.endbit >= BITS_PER_BYTE * __f.size, \
+			 __stringify(fields) " field " __stringify(index) \
+			 " exceeds unpacked storage size"); \
+	__builtin_choose_expr(index != 0, \
+			      CHECK_PACKED_FIELD_OVERLAP(fields, index - 1, index), \
+			      1); \
 })
 
 #define CHECK_PACKED_FIELDS_SIZE(fields, pbuflen) ({ \

diff --git a/scripts/gen_packed_field_checks.c b/scripts/gen_packed_field_checks.c
index 09a21afd640b..fabbb741c9a8 100644
--- a/scripts/gen_packed_field_checks.c
+++ b/scripts/gen_packed_field_checks.c

@@ -9,15 +9,9 @@ int main(int argc, char **argv)
 {
 	for (int i = 1; i <= MAX_PACKED_FIELD_SIZE; i++) {
 		printf("#define CHECK_PACKED_FIELDS_%d(fields) ({ \\\n", i);
-		printf("\ttypeof(&(fields)[0]) _f = (fields); \\\n");
-		printf("\tBUILD_BUG_ON(ARRAY_SIZE(fields) != %d); \\\n", i);
 
 		for (int j = 0; j < i; j++)
-			printf("\tCHECK_PACKED_FIELD(_f[%d]); \\\n", j);
-
-		for (int j = 1; j < i; j++)
-			printf("\tCHECK_PACKED_FIELD_OVERLAP(_f[0].startbit < _f[1].startbit, _f[%d], _f[%d]); \\\n",
-			       j - 1, j);
+			printf("\tCHECK_PACKED_FIELD(fields, %d); \\\n", j);
 
 		printf("})\n\n");
 	}

And there's one more thing I tried, which mostly worked. That was to
express CHECK_PACKED_FIELDS_N in terms of CHECK_PACKED_FIELDS_N-1.
This further reduced the auto-generated code size from 1478 lines to 302
lines, which I think is appealing.

diff --git a/scripts/gen_packed_field_checks.c b/scripts/gen_packed_field_checks.c
index fabbb741c9a8..bac85c04ef20 100644
--- a/scripts/gen_packed_field_checks.c
+++ b/scripts/gen_packed_field_checks.c

@@ -10,9 +10,10 @@ int main(int argc, char **argv)
 	for (int i = 1; i <= MAX_PACKED_FIELD_SIZE; i++) {
 		printf("#define CHECK_PACKED_FIELDS_%d(fields) ({ \\\n", i);
 
-		for (int j = 0; j < i; j++)
-			printf("\tCHECK_PACKED_FIELD(fields, %d); \\\n", j);
+		if (i != 1)
+			printf("\tCHECK_PACKED_FIELDS_%d(fields); \\\n", i - 1);
 
+		printf("\tCHECK_PACKED_FIELD(fields, %d); \\\n", i);
 		printf("})\n\n");
 	}
 

The problem is that, for some reason, it introduces this sparse warning:

../lib/packing_test.c:436:9: warning: invalid access past the end of 'test_fields' (24 24)
../lib/packing_test.c:448:9: warning: invalid access past the end of 'test_fields' (24 24)