Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/13573


Change subject: add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use
......................................................................

add osmo_{escape,quote}_str_buf2() for OSMO_STRBUF_APPEND() use

To be able to append an escaped or quoted string using OSMO_STRBUF_APPEND(),
the function signature must be like snprintf: buf and size are first arguments,
return value is characters-that-would-be-written.

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

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.

Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
---
M TODO-RELEASE
M include/osmocom/core/utils.h
M src/utils.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 110 insertions(+), 44 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/73/13573/1

diff --git a/TODO-RELEASE b/TODO-RELEASE
index 5ddc57a..7c81e32 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -11,3 +11,11 @@
 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 e19649a..ecb04b5 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -138,9 +138,11 @@
 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);
+const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
+int osmo_escape_str_buf2(char *buf, size_t bufsize, 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);
+const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t 
bufsize);
+int osmo_quote_str_buf2(char *buf, size_t bufsize, const char *str, int 
in_len);

 uint32_t osmo_isqrt32(uint32_t x);

diff --git a/src/utils.c b/src/utils.c
index b50ceab..7def0b8 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -557,22 +557,74 @@
  * \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)
 {
-       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;
-
+       int rc;
        if (!str)
                return "(null)";

+       rc = osmo_escape_str_buf2(buf, bufsize, str, in_len);
+       if (!buf || rc < 0)
+               return "(error)";
+       return buf;
+}
+
+/*! 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.
+ * The function signature is suitable for OSMO_STRBUF_APPEND().
+ * \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()).
+ */
+int 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;
+
+       if (!str)
+               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])
@@ -580,24 +632,16 @@
                     && str[next_unprintable] != '\\';
                     next_unprintable++);

-               if (next_unprintable == in_len && in_pos == 0) {
-                       osmo_strlcpy(buf, str, bufsize);
-                       return buf;
-               }
+               OSMO_STRBUF_APPEND(sb, osmo_print_n, &str[in_pos], 
next_unprintable - in_pos);
+               in_pos = next_unprintable;

-               while (in_pos < next_unprintable && out_pos < out_len)
-                       out[out_pos++] = str[in_pos++];
-
-               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');
@@ -613,19 +657,14 @@
 #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 sb.chars_needed;
 }

 /*! Return the string with all non-printable characters escaped.
@@ -639,24 +678,41 @@
        return osmo_escape_str_buf(str, in_len, namebuf, sizeof(namebuf));
 }

+/*! 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().
+ * \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()).
+ */
+int 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(sb, osmo_escape_str_buf2, str, in_len);
+               OSMO_STRBUF_PRINTF(sb, "\"");
+       }
+       return sb.chars_needed;
+}
+
 /*! Like osmo_escape_str(), 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.
  * \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;
 }

diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c
index 211b4d1..d27cdf5 100644
--- a/tests/utils/utils_test.c
+++ b/tests/utils/utils_test.c
@@ -585,7 +585,7 @@

        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 @@
        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 5783eb1..e30fc5b 100644
--- a/tests/utils/utils_test.ok
+++ b/tests/utils/utils_test.ok
@@ -258,11 +258,11 @@
 - 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'


--
To view, visit https://gerrit.osmocom.org/13573
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
Gerrit-Change-Number: 13573
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de>

Reply via email to