<p>Harald Welte has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/13447">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">osmo_escape_str_buf: Always copy, don't return input string pointer<br><br>osmo_escape_str_buf() used to have the somewhat odd semantics that<br>if no escaping was needed, it would return the original pointe without<br>making any copy to the output buffer. While this seems like a elegant<br>optimization, it is a very strange behavior and it works differently<br>than all of our other *_buf() functions. Let's unify the API and<br>turn osmo_escape_str_buf() into a strlcpy() if no escaping is needed.<br><br>Change-Id: I3a02bdb27008a73101c2db41ac04248960ed4064<br>---<br>M include/osmocom/core/utils.h<br>M src/utils.c<br>M tests/utils/utils_test.c<br>M tests/utils/utils_test.ok<br>4 files changed, 10 insertions(+), 31 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/47/13447/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h</span><br><span>index 16159d3..e3728cd 100644</span><br><span>--- a/include/osmocom/core/utils.h</span><br><span>+++ b/include/osmocom/core/utils.h</span><br><span>@@ -137,7 +137,7 @@</span><br><span> bool osmo_separated_identifiers_valid(const char *str, const char *sep_chars);</span><br><span> </span><br><span> const char *osmo_escape_str(const char *str, int len);</span><br><span style="color: hsl(0, 100%, 40%);">-const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize);</span><br><span style="color: hsl(120, 100%, 40%);">+char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize);</span><br><span> const char *osmo_quote_str(const char *str, int in_len);</span><br><span> const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize);</span><br><span> </span><br><span>diff --git a/src/utils.c b/src/utils.c</span><br><span>index 2d5bcb0..019e733 100644</span><br><span>--- a/src/utils.c</span><br><span>+++ b/src/utils.c</span><br><span>@@ -538,14 +538,14 @@</span><br><span> return osmo_separated_identifiers_valid(str, NULL);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*! Return the string with all non-printable characters escaped.</span><br><span style="color: hsl(120, 100%, 40%);">+/*! Return the string with all non-printable characters escapeda, in user-supplied buffer.</span><br><span> * \param[in] str A string that may contain any characters.</span><br><span> * \param[in] len Pass -1 to print until nul char, or >= 0 to force a length.</span><br><span> * \param[inout] buf string buffer to write escaped characters to.</span><br><span> * \param[in] bufsize size of \a buf.</span><br><span style="color: hsl(0, 100%, 40%);">- * \returns buf containing an escaped representation, possibly truncated, or str itself.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \returns buf containing an escaped representation, possibly truncated.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">-const char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize)</span><br><span style="color: hsl(120, 100%, 40%);">+char *osmo_escape_str_buf(const char *str, int in_len, char *buf, size_t bufsize)</span><br><span> {</span><br><span> int in_pos = 0;</span><br><span> int next_unprintable = 0;</span><br><span>@@ -567,9 +567,10 @@</span><br><span> && str[next_unprintable] != '\\';</span><br><span> next_unprintable++);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (next_unprintable == in_len</span><br><span style="color: hsl(0, 100%, 40%);">- && in_pos == 0)</span><br><span style="color: hsl(0, 100%, 40%);">- return str;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (next_unprintable == in_len && in_pos == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_strlcpy(buf, str, bufsize);</span><br><span style="color: hsl(120, 100%, 40%);">+ return buf;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> while (in_pos < next_unprintable && out_pos < out_len)</span><br><span> out[out_pos++] = str[in_pos++];</span><br><span>@@ -633,25 +634,13 @@</span><br><span> */</span><br><span> const char *osmo_quote_str_buf(const char *str, int in_len, char *buf, size_t bufsize)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- const char *res;</span><br><span> int l;</span><br><span> if (!str)</span><br><span> return "NULL";</span><br><span> if (bufsize < 3)</span><br><span> return "<buf-too-small>";</span><br><span> buf[0] = '"';</span><br><span style="color: hsl(0, 100%, 40%);">- res = osmo_escape_str_buf(str, in_len, buf + 1, bufsize - 2);</span><br><span style="color: hsl(0, 100%, 40%);">- /* if osmo_escape_str_buf() returned the str itself, we need to copy it to buf to be able to</span><br><span style="color: hsl(0, 100%, 40%);">- * quote it. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (res == str) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* max_len = bufsize - two quotes - nul term */</span><br><span style="color: hsl(0, 100%, 40%);">- int max_len = bufsize - 2 - 1;</span><br><span style="color: hsl(0, 100%, 40%);">- if (in_len >= 0)</span><br><span style="color: hsl(0, 100%, 40%);">- max_len = OSMO_MIN(in_len, max_len);</span><br><span style="color: hsl(0, 100%, 40%);">- /* It is not allowed to pass unterminated strings into osmo_strlcpy() :/ */</span><br><span style="color: hsl(0, 100%, 40%);">- strncpy(buf + 1, str, max_len);</span><br><span style="color: hsl(0, 100%, 40%);">- buf[1 + max_len] = '\0';</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_escape_str_buf(str, in_len, buf + 1, bufsize - 2);</span><br><span> l = strlen(buf);</span><br><span> buf[l] = '"';</span><br><span> buf[l+1] = '\0'; /* both osmo_escape_str_buf() and max_len above ensure room for '\0' */</span><br><span>diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c</span><br><span>index d592fe0..711d6e1 100644</span><br><span>--- a/tests/utils/utils_test.c</span><br><span>+++ b/tests/utils/utils_test.c</span><br><span>@@ -545,7 +545,7 @@</span><br><span> </span><br><span> printf("- passthru:\n");</span><br><span> res = osmo_escape_str(printable, -1);</span><br><span style="color: hsl(0, 100%, 40%);">- if (res != printable)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (strcmp(res, printable))</span><br><span> printf("NOT passed through! \"%s\"\n", res);</span><br><span> else</span><br><span> printf("passed through unchanged \"%s\"\n", res);</span><br><span>@@ -560,14 +560,6 @@</span><br><span> memset(out_buf, 0x7f, sizeof(out_buf));</span><br><span> printf("\"%s\"\n", osmo_escape_str_buf((const char *)in_buf, sizeof(in_buf), out_buf, 10));</span><br><span> OSMO_ASSERT(out_buf[10] == 0x7f);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- printf("- passthrough without truncation when no escaping needed:\n");</span><br><span style="color: hsl(0, 100%, 40%);">- memset(in_buf, 'x', sizeof(in_buf));</span><br><span style="color: hsl(0, 100%, 40%);">- in_buf[19] = 'E';</span><br><span style="color: hsl(0, 100%, 40%);">- in_buf[20] = '\0';</span><br><span style="color: hsl(0, 100%, 40%);">- memset(out_buf, 0x7f, sizeof(out_buf));</span><br><span style="color: hsl(0, 100%, 40%);">- printf("\"%s\"\n", osmo_escape_str_buf((const char *)in_buf, -1, out_buf, 10));</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_ASSERT(out_buf[0] == 0x7f);</span><br><span> }</span><br><span> </span><br><span> static void str_quote_test(void)</span><br><span>diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok</span><br><span>index 1215ddd..8fce9f1 100644</span><br><span>--- a/tests/utils/utils_test.ok</span><br><span>+++ b/tests/utils/utils_test.ok</span><br><span>@@ -232,8 +232,6 @@</span><br><span> ""</span><br><span> - truncation when too long:</span><br><span> "\axxxxxxE"</span><br><span style="color: hsl(0, 100%, 40%);">-- passthrough without truncation when no escaping needed:</span><br><span style="color: hsl(0, 100%, 40%);">-"xxxxxxxxxxxxxxxxxxxE"</span><br><span> </span><br><span> Testing string quoting</span><br><span> - all chars from 0 to 255 in batches of 16:</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/13447">change 13447</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/13447"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I3a02bdb27008a73101c2db41ac04248960ed4064 </div>
<div style="display:none"> Gerrit-Change-Number: 13447 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Harald Welte <laforge@gnumonks.org> </div>