lib: os: cbprintf: Add CBPRINTF_PACKAGE_COPY_KEEP_RO_STR flag

Add flag to copy function which indicates that read-only
string locations shall be kept in the output package.

Updated cbprintf_package test to pass.

Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>

include/sys/cbprintf.h

@@ -131,9 +131,18 @@ BUILD_ASSERT(Z_IS_POW2(CBPRINTF_PACKAGE_ALIGNMENT));
  * is set, list of read-write strings is examined and if they are not determined
  * to be read-only, they are copied into the destination package.
  * If @ref CBPRINTF_PACKAGE_COPY_RO_STR is not set, remaining string locations
- * are considered as pointing to read-only location.
+ * are considered as pointing to read-only location and they are copy to the
+ * package if @ref CBPRINTF_PACKAGE_COPY_KEEP_RO_STR is set.
  */
 #define CBPRINTF_PACKAGE_COPY_RW_STR BIT(1)
+
+/** @brief Keep read-only location indexes in the package.
+ *
+ * If it is set read-only string pointers are kept in the package after copy. If
+ * not set they are discarded.
+ */
+#define CBPRINTF_PACKAGE_COPY_KEEP_RO_STR BIT(2)
+
 /**@} */
 
 /** @brief Signature for a cbprintf callback function.

lib/os/cbprintf_packaged.c

@@ -733,10 +733,6 @@ int cbprintf_package_copy(void *in_packaged,
 
 	in_len != 0 ? in_len : get_package_len(in_packaged);
 
-	if (packaged && (len < in_len)) {
-		return -ENOSPC;
-	}
-
 	/* Get number of RO string indexes in the package and check if copying
 	 * includes appending those strings.
 	 */
@@ -793,9 +789,15 @@ int cbprintf_package_copy(void *in_packaged,
 				str_pos++;
 			}
 		} else {
-			str_pos += ros_nbr;
+			if (ros_nbr && flags & CBPRINTF_PACKAGE_COPY_KEEP_RO_STR) {
+				str_pos += ros_nbr;
+			}
 		}
 
+		bool drop_ro_str_pos = !(flags &
+					(CBPRINTF_PACKAGE_COPY_KEEP_RO_STR |
+					 CBPRINTF_PACKAGE_COPY_RO_STR));
+
 		/* Handle RW strings. */
 		for (int i = 0; i < rws_nbr; i++) {
 			const char *str = *(const char **)&buf32[*str_pos];
@@ -811,6 +813,14 @@ int cbprintf_package_copy(void *in_packaged,
 				}
 				out_len += len;
 			}
+
+			if (is_ro && drop_ro_str_pos) {
+				/* If read-only string location is dropped decreased
+				 * length.
+				 */
+				out_len--;
+			}
+
 			str_pos++;
 		}
 
@@ -825,6 +835,9 @@ int cbprintf_package_copy(void *in_packaged,
 	memcpy(dst, in_packaged, args_size);
 	dst += args_size;
 
+	/* Pointer to the beginning of string locations in the destination package. */
+	uint8_t *dst_str_loc = dst;
+
 	/* If read-only strings shall be appended to the output package copy
 	 * their indexes to the local array, otherwise indicate that indexes
 	 * shall remain in the output package.
@@ -837,10 +850,12 @@ int cbprintf_package_copy(void *in_packaged,
 		str_pos += ros_nbr;
 	} else {
 		scpy_cnt = 0;
-		if (ros_nbr) {
+		if (ros_nbr && flags & CBPRINTF_PACKAGE_COPY_KEEP_RO_STR) {
 			memcpy(dst, str_pos, ros_nbr);
 			dst += ros_nbr;
 			str_pos += ros_nbr;
+		} else {
+			dst_hdr[2] = 0;
 		}
 	}
 
@@ -852,21 +867,30 @@ int cbprintf_package_copy(void *in_packaged,
 		const char *str = *(const char **)&buf32[*str_pos];
 		bool is_ro = ptr_in_rodata(str);
 
-		if ((is_ro && flags & CBPRINTF_PACKAGE_COPY_RO_STR) ||
+		if (is_ro) {
-		    (!is_ro && flags & CBPRINTF_PACKAGE_COPY_RW_STR)) {
+			if (flags & CBPRINTF_PACKAGE_COPY_RO_STR) {
-			cpy_str_pos[scpy_cnt++] = *str_pos;
+				cpy_str_pos[scpy_cnt++] = *str_pos;
+			} else if (flags & CBPRINTF_PACKAGE_COPY_KEEP_RO_STR) {
+				*dst++ = *str_pos;
+				/* Increment amount of ro locations. */
+				dst_hdr[2]++;
+			} else {
+				/* Drop information about ro_str location. */
+			}
 		} else {
-			*dst++ = *str_pos;
+			if (flags & CBPRINTF_PACKAGE_COPY_RW_STR) {
+				cpy_str_pos[scpy_cnt++] = *str_pos;
+			} else {
+				*dst++ = *str_pos;
+			}
 		}
 		str_pos++;
 	}
 
-	uint8_t out_str_pos_nbr = ros_nbr + rws_nbr - scpy_cnt;
-
 	/* Increment amount of strings appended to the package. */
 	dst_hdr[1] += scpy_cnt;
 	/* Update number of rw string locations in the package. */
-	dst_hdr[3] = out_str_pos_nbr - dst_hdr[2];
+	dst_hdr[3] = (uint8_t)(uintptr_t)(dst - dst_str_loc) - dst_hdr[2];
 
 	/* Copy appended strings from source package to destination. */
 	size_t strs_len = in_len - (args_size + ros_nbr + rws_nbr);
@@ -880,7 +904,7 @@ int cbprintf_package_copy(void *in_packaged,
 	}
 
 	/* Calculate remaining space in the buffer. */
-	size_t rem = len - (args_size + strs_len + out_str_pos_nbr);
+	size_t rem = len - ((size_t)(uintptr_t)(dst - dst_hdr));
 
 	if (rem <= scpy_cnt) {
 		return -ENOSPC;

tests/lib/cbprintf_package/src/main.c

@@ -525,9 +525,12 @@ static void test_cbprintf_ro_rw_loc(void)
 	check_package(package, len, exp_str);
 	check_package(cpackage, clen, exp_str);
 
+	uint32_t cpy_flags = CBPRINTF_PACKAGE_COPY_RW_STR |
+			     CBPRINTF_PACKAGE_COPY_KEEP_RO_STR;
+
 	/* Calculate size needed for package with appended read-write strings. */
 	clen = cbprintf_package_copy(package, sizeof(package), NULL, 0,
-				     CBPRINTF_PACKAGE_COPY_RW_STR, NULL, 0);
+				     cpy_flags, NULL, 0);
 
 	/* Length will be increased by 2 string lengths + null terminators. */
 	zassert_equal(clen, len + (int)strlen(test_str1) + (int)strlen(test_str2) + 2, NULL);
@@ -535,7 +538,7 @@ static void test_cbprintf_ro_rw_loc(void)
 	uint8_t __aligned(CBPRINTF_PACKAGE_ALIGNMENT) cpackage2[clen];
 
 	clen2 = cbprintf_package_copy(package, sizeof(package), cpackage2, sizeof(cpackage2),
-				     CBPRINTF_PACKAGE_COPY_RW_STR, NULL, 0);
+				      cpy_flags, NULL, 0);
 
 	zassert_equal(clen, clen2, NULL);
 
@@ -717,9 +720,12 @@ static void test_cbprintf_rw_loc_const_char_ptr(void)
 	zassert_equal(hdr[2], 0, NULL);
 	zassert_equal(hdr[3], 2, NULL);
 
+	uint32_t cpy_flags = CBPRINTF_PACKAGE_COPY_RW_STR |
+			     CBPRINTF_PACKAGE_COPY_KEEP_RO_STR;
+
 	/* Calculate size needed for package with appended read-only strings. */
 	clen = cbprintf_package_copy(spackage, sizeof(spackage), NULL, 0,
-				     CBPRINTF_PACKAGE_COPY_RW_STR, NULL, 0);
+				     cpy_flags, NULL, 0);
 
 	/* Length will be increased by 2 string lengths + null terminators. */
 	zassert_equal(clen, slen + (int)strlen(test_str1) + 1, NULL);
@@ -727,7 +733,7 @@ static void test_cbprintf_rw_loc_const_char_ptr(void)
 	uint8_t __aligned(CBPRINTF_PACKAGE_ALIGNMENT) cpackage[clen];
 
 	clen2 = cbprintf_package_copy(spackage, sizeof(spackage), cpackage, sizeof(cpackage),
-				     CBPRINTF_PACKAGE_COPY_RW_STR, NULL, 0);
+				     cpy_flags, NULL, 0);
 
 	zassert_equal(clen, clen2, NULL);
 
@@ -735,8 +741,8 @@ static void test_cbprintf_rw_loc_const_char_ptr(void)
 
 	/* Check that one string has been appended. */
 	zassert_equal(hdr[1], 1, NULL);
-	zassert_equal(hdr[2], 0, NULL);
+	zassert_equal(hdr[2], 1, NULL);
-	zassert_equal(hdr[3], 1, NULL);
+	zassert_equal(hdr[3], 0, NULL);
 
 	check_package(spackage, slen, exp_str);
 	test_str1[0] = '\0';