add osmo_{escape,quote}_str_buf2() for standard args ordering

To be able to append an escaped or quoted string using
OSMO_STRBUF_APPEND_NOLEN(), the function signature must have the buf and len as
first args, like most other *_buf() functions.

Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.

A recent patch [1] has changed the return value of osmo_escape_str_buf() to
char*, removing the const. However, the functions may return const strings,
hence re-add the const. The new signatures always return the non-const buffer.

To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.

I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:

(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
  "this string is trunca"
and now this becomes, sic:
  "this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.

(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
  their content (i.e. code that usually writes out strings in full length
  "suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.

Adjust tests to accomodate above changes.

[1] 4a62eda225ab7f3c9556990c81a6fc5e19b5eec8
    Ibf85f79e93244f53b2684ff6f1095c5b41203e05

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae

5 files changed, 115 insertions, 47 deletions

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 5ddc57a3..7c81e323 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -11,3 +11,11 @@ libosmogb	gprs_ns_inst		Adding bss_sns_fi member for IP-SNS support
 libosmogb	gprs_nsvc		Adding sig_weight and data_weight members for IP-SNS support
 libosmogb	various new symbols	Adding functions related to IP-SNS support
 libosmocore	osmo_fsm_inst		Add flag proc.terminating (ABI change)
+libosmocore	osmo_escape_str(),      These now always copy to the buffer instead of returning the
+		osmo_escape_str_buf()	  unchanged input string when no chars needed escaping, hence
+		                          returned strings might now also be truncated even if all chars were printable.
+libosmocore	osmo_escape_str_buf2()	New function signature similar to snprintf(), for use with OSMO_STRBUF_APPEND().
+libosmocore	osmo_quote_str(),       On string truncation, these used to print a closing quote '"' after the
+		osmo_quote_str_buf()	  truncated string. This is no longer the case. e.g. a string 'truncated' in a
+					  9-char buffer used to print '"trunca"\0', which now becomes '"truncat\0'.
+libosmocore	osmo_quote_str_buf2()	New function signature similar to snprintf(), for use with OSMO_STRBUF_APPEND().
diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index f13c1e48..08735fdb 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -142,12 +142,16 @@ bool osmo_identifier_valid(const char *str);
 bool osmo_separated_identifiers_valid(const char *str, const char *sep_chars);
 
 const char *osmo_escape_str(const char *str, int len);
-char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize);
+char *osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int in_len);
+const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize);
 char *osmo_escape_str_c(const void *ctx, const char *str, int in_len);
 const char *osmo_quote_str(const char *str, int in_len);
-char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize);
+char *osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int in_len);
+const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize);
 char *osmo_quote_str_c(const void *ctx, const char *str, int in_len);
 
+int osmo_print_n(char *buf, size_t bufsize, const char *str, size_t n);
+
 uint32_t osmo_isqrt32(uint32_t x);
 
 const char osmo_luhn(const char* in, int in_len);
diff --git a/src/utils.c b/src/utils.c
index 9ab990ab..6116d3ad 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -594,28 +594,78 @@ bool osmo_identifier_valid(const char *str)
 	return osmo_separated_identifiers_valid(str, NULL);
 }
 
-/*! Return the string with all non-printable characters escapeda, in user-supplied buffer.
+/*! Like osmo_escape_str_buf2, but with unusual ordering of arguments, and may sometimes return string constants instead
+ * of writing to buf for error cases or empty input.
+ * Most *_buf() functions have the buffer and size as first arguments, here the arguments are last.
+ * In particular, this function signature doesn't work with OSMO_STRBUF_APPEND_NOLEN().
  * \param[in] str  A string that may contain any characters.
  * \param[in] len  Pass -1 to print until nul char, or >= 0 to force a length.
  * \param[inout] buf  string buffer to write escaped characters to.
  * \param[in] bufsize  size of \a buf.
- * \returns buf containing an escaped representation, possibly truncated.
+ * \returns buf containing an escaped representation, possibly truncated,
+ *          or "(null)" if str == NULL, or "(error)" in case of errors.
  */
-char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize)
+const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize)
 {
+	if (!str)
+		return "(null)";
+	if (!buf || !bufsize)
+		return "(error)";
+	return osmo_escape_str_buf2(buf, bufsize, str, in_len);
+}
+
+/*! Copy N characters to a buffer with a function signature useful for OSMO_STRBUF_APPEND().
+ * Similarly to snprintf(), the result is always nul terminated (except if buf is NULL or bufsize is 0).
+ * \param[out] buf  Target buffer.
+ * \param[in] bufsize  sizeof(buf).
+ * \param[in] str  String to copy.
+ * \param[in] n  Maximum number of non-nul characters to copy.
+ * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()).
+ */
+int osmo_print_n(char *buf, size_t bufsize, const char *str, size_t n)
+{
+	size_t write_n;
+
+	if (!str)
+		str = "";
+
+	n = strnlen(str, n);
+
+	if (!buf || !bufsize)
+		return n;
+	write_n = n;
+	if (write_n >= bufsize)
+		write_n = bufsize - 1;
+	if (write_n)
+		strncpy(buf, str, write_n);
+	buf[write_n] = '\0';
+
+	return n;
+}
+
+/*! Return the string with all non-printable characters escaped.
+ * \param[out] buf  string buffer to write escaped characters to.
+ * \param[in] bufsize  sizeof(buf).
+ * \param[in] str  A string that may contain any characters.
+ * \param[in] in_len  Pass -1 to print until nul char, or >= 0 to force a length (also past nul chars).
+ * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()).
+ */
+char *osmo_escape_str_buf2(char *buf, size_t bufsize, const char *str, int in_len)
+{
+	struct osmo_strbuf sb = { .buf = buf, .len = bufsize };
 	int in_pos = 0;
 	int next_unprintable = 0;
-	int out_pos = 0;
-	char *out = buf;
-	/* -1 to leave space for a final \0 */
-	int out_len = bufsize-1;
 
 	if (!str)
-		return "(null)";
+		in_len = 0;
 
 	if (in_len < 0)
 		in_len = strlen(str);
 
+	/* Make sure of '\0' termination */
+	if (!in_len)
+		OSMO_STRBUF_PRINTF(sb, "%s", "");
+
 	while (in_pos < in_len) {
 		for (next_unprintable = in_pos;
 		     next_unprintable < in_len && isprint((int)str[next_unprintable])
@@ -623,24 +673,16 @@ char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize
 		     && str[next_unprintable] != '\\';
 		     next_unprintable++);
 
-		if (next_unprintable == in_len && in_pos == 0) {
-			osmo_strlcpy(buf, str, bufsize);
-			return buf;
-		}
-
-		while (in_pos < next_unprintable && out_pos < out_len)
-			out[out_pos++] = str[in_pos++];
+		OSMO_STRBUF_APPEND(sb, osmo_print_n, &str[in_pos], next_unprintable - in_pos);
+		in_pos = next_unprintable;
 
-		if (out_pos == out_len || in_pos == in_len)
+		if (in_pos == in_len)
 			goto done;
 
 		switch (str[next_unprintable]) {
 #define BACKSLASH_CASE(c, repr) \
 		case c: \
-			if (out_pos > out_len-2) \
-				goto done; \
-			out[out_pos++] = '\\'; \
-			out[out_pos++] = repr; \
+			OSMO_STRBUF_PRINTF(sb, "\\%c", repr); \
 			break
 
 		BACKSLASH_CASE('\n', 'n');
@@ -656,19 +698,14 @@ char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize
 #undef BACKSLASH_CASE
 
 		default:
-			out_pos += snprintf(&out[out_pos], out_len - out_pos, "\\%u", (unsigned char)str[in_pos]);
-			if (out_pos > out_len) {
-				out_pos = out_len;
-				goto done;
-			}
+			OSMO_STRBUF_PRINTF(sb, "\\%u", (unsigned char)str[in_pos]);
 			break;
 		}
 		in_pos ++;
 	}
 
 done:
-	out[out_pos] = '\0';
-	return out;
+	return buf;
 }
 
 /*! Return the string with all non-printable characters escaped.
@@ -692,27 +729,46 @@ char *osmo_escape_str_c(const void *ctx, const char *str, int in_len)
 	char *buf = talloc_size(ctx, in_len+1);
 	if (!buf)
 		return NULL;
-	return osmo_escape_str_buf(str, in_len, buf, in_len+1);
+	return osmo_escape_str_buf2(buf, in_len+1, str, in_len);
 }
 
-/*! Like osmo_escape_str(), but returns double-quotes around a string, or "NULL" for a NULL string.
+/*! Like osmo_escape_str_buf2(), but returns double-quotes around a string, or "NULL" for a NULL string.
  * This allows passing any char* value and get its C representation as string.
+ * The function signature is suitable for OSMO_STRBUF_APPEND_NOLEN().
+ * \param[out] buf  string buffer to write escaped characters to.
+ * \param[in] bufsize  sizeof(buf).
+ * \param[in] str  A string that may contain any characters.
+ * \param[in] in_len  Pass -1 to print until nul char, or >= 0 to force a length.
+ * \return Number of characters that would be written if bufsize were large enough excluding '\0' (like snprintf()).
+ */
+char *osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int in_len)
+{
+	struct osmo_strbuf sb = { .buf = buf, .len = bufsize };
+	if (!str)
+		OSMO_STRBUF_PRINTF(sb, "NULL");
+	else {
+		OSMO_STRBUF_PRINTF(sb, "\"");
+		OSMO_STRBUF_APPEND_NOLEN(sb, osmo_escape_str_buf2, str, in_len);
+		OSMO_STRBUF_PRINTF(sb, "\"");
+	}
+	return buf;
+}
+
+/*! Like osmo_quote_str_buf2, but with unusual ordering of arguments, and may sometimes return string constants instead
+ * of writing to buf for error cases or empty input.
+ * Most *_buf() functions have the buffer and size as first arguments, here the arguments are last.
+ * In particular, this function signature doesn't work with OSMO_STRBUF_APPEND_NOLEN().
  * \param[in] str  A string that may contain any characters.
  * \param[in] in_len  Pass -1 to print until nul char, or >= 0 to force a length.
  * \returns buf containing a quoted and escaped representation, possibly truncated.
  */
-char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize)
+const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize)
 {
-	int l;
 	if (!str)
 		return "NULL";
-	if (bufsize < 3)
-		return "<buf-too-small>";
-	buf[0] = '"';
-	osmo_escape_str_buf(str, in_len, buf + 1, bufsize - 2);
-	l = strlen(buf);
-	buf[l] = '"';
-	buf[l+1] = '\0'; /* both osmo_escape_str_buf() and max_len above ensure room for '\0' */
+	if (!buf || !bufsize)
+		return "(error)";
+	osmo_quote_str_buf2(buf, bufsize, str, in_len);
 	return buf;
 }
 
@@ -738,7 +794,7 @@ char *osmo_quote_str_c(const void *ctx, const char *str, int in_len)
 	char *buf = talloc_size(ctx, OSMO_MAX(in_len+2, 32));
 	if (!buf)
 		return NULL;
-	return osmo_quote_str_buf(str, in_len, buf, 32);
+	return osmo_quote_str_buf2(buf, 32, str, in_len);
 }
 
 /*! perform an integer square root operation on unsigned 32bit integer.
diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c
index 223f67d2..70d017fe 100644
--- a/tests/utils/utils_test.c
+++ b/tests/utils/utils_test.c
@@ -585,7 +585,7 @@ static void str_quote_test(void)
 
 	printf("- never passthru:\n");
 	res = osmo_quote_str(printable, -1);
-	if (res != printable)
+	if (strcmp(res, printable))
 		printf("NOT passed through. '%s'\n", res);
 	else
 		printf("passed through unchanged '%s'\n", res);
@@ -596,14 +596,14 @@ static void str_quote_test(void)
 	printf("- truncation when too long:\n");
 	memset(in_buf, 'x', sizeof(in_buf));
 	in_buf[0] = '\a';
-	in_buf[5] = 'E';
+	in_buf[6] = 'E';
 	memset(out_buf, 0x7f, sizeof(out_buf));
 	printf("'%s'\n", osmo_quote_str_buf((const char *)in_buf, sizeof(in_buf), out_buf, 10));
 	OSMO_ASSERT(out_buf[10] == 0x7f);
 
 	printf("- always truncation, even when no escaping needed:\n");
 	memset(in_buf, 'x', sizeof(in_buf));
-	in_buf[6] = 'E'; /* dst has 10, less 2 quotes and nul, leaves 7, i.e. in[6] is last */
+	in_buf[7] = 'E'; /* dst has 10, less 1 quote and nul, leaves 8, i.e. in[7] is last */
 	in_buf[20] = '\0';
 	memset(out_buf, 0x7f, sizeof(out_buf));
 	printf("'%s'\n", osmo_quote_str_buf((const char *)in_buf, -1, out_buf, 10));
diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok
index 587c6f09..c150a8d0 100644
--- a/tests/utils/utils_test.ok
+++ b/tests/utils/utils_test.ok
@@ -258,11 +258,11 @@ NOT passed through. '"printable"'
 - zero length:
 '""'
 - truncation when too long:
-'"\axxxxE"'
+'"\axxxxxE'
 - always truncation, even when no escaping needed:
-'"xxxxxxE"'
+'"xxxxxxxE'
 - try to feed too little buf for quoting:
-'<buf-too-small>'
+'"'
 - NULL string becomes a "NULL" literal:
 'NULL'