eal: support strlcat function
Checks
Commit Message
Add the strlcat function to DPDK to exist alongside the strlcpy one. While
strncat is generally safe for use for concatenation, the API for the
strlcat function is perhaps a little nicer to use, and supports truncation
detection.
See commit: 5364de644a4b ("eal: support strlcpy function") for more
details on the function selection logic, since we only should be using the
DPDK-provided version when no system-provided version is present.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
.../common/include/rte_string_fns.h | 15 +++++++
test/test/test_string_fns.c | 45 +++++++++++++++++++
2 files changed, 60 insertions(+)
Comments
On 16-Jan-19 12:48 PM, Bruce Richardson wrote:
> Add the strlcat function to DPDK to exist alongside the strlcpy one. While
> strncat is generally safe for use for concatenation, the API for the
> strlcat function is perhaps a little nicer to use, and supports truncation
> detection.
>
> See commit: 5364de644a4b ("eal: support strlcpy function") for more
> details on the function selection logic, since we only should be using the
> DPDK-provided version when no system-provided version is present.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
<...>
> static int
> test_string_fns(void)
> {
> if (test_rte_strsplit() < 0)
> return -1;
> + if (test_rte_strlcat() < 0)
> + return -1;
> return 0;
> }
>
>
Unrelated, but do we also need to test strlcpy, strscpy and other
functions that were introduced?
On Thu, Jan 17, 2019 at 10:39:02AM +0000, Burakov, Anatoly wrote:
> On 16-Jan-19 12:48 PM, Bruce Richardson wrote:
> > Add the strlcat function to DPDK to exist alongside the strlcpy one.
> > While strncat is generally safe for use for concatenation, the API for
> > the strlcat function is perhaps a little nicer to use, and supports
> > truncation detection.
> >
> > See commit: 5364de644a4b ("eal: support strlcpy function") for more
> > details on the function selection logic, since we only should be using
> > the DPDK-provided version when no system-provided version is present.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
>
> <...>
>
> > static int test_string_fns(void) { if (test_rte_strsplit() < 0)
> > return -1; + if (test_rte_strlcat() < 0) + return -1;
> > return 0; }
> >
>
> Unrelated, but do we also need to test strlcpy, strscpy and other
> functions that were introduced?
>
Yes, I think that would be advisable. I imagine the easiest way to test
them is to do as I have here in running comparisons with a range of inputs,
especially boundary conditions, against a known-good version for platforms
that have the functions built-in.
As always, volunteers and patches welcome... :-)
/Bruce
On 17-Jan-19 11:00 AM, Bruce Richardson wrote:
> On Thu, Jan 17, 2019 at 10:39:02AM +0000, Burakov, Anatoly wrote:
>> On 16-Jan-19 12:48 PM, Bruce Richardson wrote:
>>> Add the strlcat function to DPDK to exist alongside the strlcpy one.
>>> While strncat is generally safe for use for concatenation, the API for
>>> the strlcat function is perhaps a little nicer to use, and supports
>>> truncation detection.
>>>
>>> See commit: 5364de644a4b ("eal: support strlcpy function") for more
>>> details on the function selection logic, since we only should be using
>>> the DPDK-provided version when no system-provided version is present.
>>>
>>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
>>
>> <...>
>>
>>> static int test_string_fns(void) { if (test_rte_strsplit() < 0)
>>> return -1; + if (test_rte_strlcat() < 0) + return -1;
>>> return 0; }
>>>
>>
>> Unrelated, but do we also need to test strlcpy, strscpy and other
>> functions that were introduced?
>>
>
> Yes, I think that would be advisable. I imagine the easiest way to test
> them is to do as I have here in running comparisons with a range of inputs,
> especially boundary conditions, against a known-good version for platforms
> that have the functions built-in.
> As always, volunteers and patches welcome... :-)
/action hides
>
> /Bruce
>
On 1/16/2019 12:48 PM, Bruce Richardson wrote:
> Add the strlcat function to DPDK to exist alongside the strlcpy one. While
> strncat is generally safe for use for concatenation, the API for the
> strlcat function is perhaps a little nicer to use, and supports truncation
> detection.
>
> See commit: 5364de644a4b ("eal: support strlcpy function") for more
> details on the function selection logic, since we only should be using the
> DPDK-provided version when no system-provided version is present.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> .../common/include/rte_string_fns.h | 15 +++++++
> test/test/test_string_fns.c | 45 +++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
> index 9a2a1ff90..e7a1656f0 100644
> --- a/lib/librte_eal/common/include/rte_string_fns.h
> +++ b/lib/librte_eal/common/include/rte_string_fns.h
> @@ -59,10 +59,24 @@ rte_strlcpy(char *dst, const char *src, size_t size)
> return (size_t)snprintf(dst, size, "%s", src);
> }
>
> +/**
> + * @internal
> + * DPDK-specific version of strlcat for systems without
> + * libc or libbsd copies of the function
> + */
> +static inline size_t
> +rte_strlcat(char *dst, const char *src, size_t size)
> +{
> + size_t l = strnlen(dst, size);
> + return l + ((l < size) ?
> + rte_strlcpy(&dst[l], src, size - l) : strlen(src));
I think operation is complex for ternary operation, regular if check can be
simpler, other than that looks good to me.
> +}
> +
> /* pull in a strlcpy function */
> #ifdef RTE_EXEC_ENV_BSDAPP
> #ifndef __BSD_VISIBLE /* non-standard functions are hidden */
> #define strlcpy(dst, src, size) rte_strlcpy(dst, src, size)
> +#define strlcat(dst, src, size) rte_strlcat(dst, src, size)
> #endif
>
> #else /* non-BSD platforms */
> @@ -71,6 +85,7 @@ rte_strlcpy(char *dst, const char *src, size_t size)
>
> #else /* no BSD header files, create own */
> #define strlcpy(dst, src, size) rte_strlcpy(dst, src, size)
> +#define strlcat(dst, src, size) rte_strlcat(dst, src, size)
>
> #endif /* RTE_USE_LIBBSD */
> #endif /* BSDAPP */
> diff --git a/test/test/test_string_fns.c b/test/test/test_string_fns.c
> index 3f091ab92..3bd8ed5d8 100644
> --- a/test/test/test_string_fns.c
> +++ b/test/test/test_string_fns.c
> @@ -129,11 +129,56 @@ test_rte_strsplit(void)
> return 0;
> }
>
> +int
> +test_rte_strlcat(void)
> +{
> + /* only run actual unit tests if we have system-provided strlcat */
> +#if defined(__BSD_VISIBLE) || defined(RTE_USE_LIBBSD)
> +#define BUF_LEN 32
> + const char dst[BUF_LEN] = "Test string";
> + const char src[] = " appended";
> + char bsd_dst[BUF_LEN];
> + char rte_dst[BUF_LEN];
> + size_t i, bsd_ret, rte_ret;
> +
> + LOG("dst = '%s', strlen(dst) = %zu\n", dst, strlen(dst));
> + LOG("src = '%s', strlen(src) = %zu\n", src, strlen(src));
> + LOG("---\n");
> +
> + for (i = 0; i < BUF_LEN; i++) {
> + /* initialize destination buffers */
> + memcpy(bsd_dst, dst, BUF_LEN);
> + memcpy(rte_dst, dst, BUF_LEN);
> + /* compare implementations */
> + bsd_ret = strlcat(bsd_dst, src, i);
> + rte_ret = rte_strlcat(rte_dst, src, i);
> + if (bsd_ret != rte_ret) {
> + LOG("Incorrect retval for buf length = %zu\n", i);
> + LOG("BSD: '%zu', rte: '%zu'\n", bsd_ret, rte_ret);
> + return -1;
> + }
> + if (memcmp(bsd_dst, rte_dst, BUF_LEN) != 0) {
> + LOG("Resulting buffers don't match\n");
> + LOG("BSD: '%s', rte: '%s'\n", bsd_dst, rte_dst);
> + return -1;
> + }
> + LOG("buffer size = %zu: dst = '%s', ret = %zu\n",
> + i, rte_dst, rte_ret);
> + }
> + LOG("Checked %zu combinations\n", i);
> +#undef BUF_LEN
> +#endif /* defined(__BSD_VISIBLE) || defined(RTE_USE_LIBBSD) */
> +
> + return 0;
> +}
> +
> static int
> test_string_fns(void)
> {
> if (test_rte_strsplit() < 0)
> return -1;
> + if (test_rte_strlcat() < 0)
> + return -1;
> return 0;
> }
>
>
@@ -59,10 +59,24 @@ rte_strlcpy(char *dst, const char *src, size_t size)
return (size_t)snprintf(dst, size, "%s", src);
}
+/**
+ * @internal
+ * DPDK-specific version of strlcat for systems without
+ * libc or libbsd copies of the function
+ */
+static inline size_t
+rte_strlcat(char *dst, const char *src, size_t size)
+{
+ size_t l = strnlen(dst, size);
+ return l + ((l < size) ?
+ rte_strlcpy(&dst[l], src, size - l) : strlen(src));
+}
+
/* pull in a strlcpy function */
#ifdef RTE_EXEC_ENV_BSDAPP
#ifndef __BSD_VISIBLE /* non-standard functions are hidden */
#define strlcpy(dst, src, size) rte_strlcpy(dst, src, size)
+#define strlcat(dst, src, size) rte_strlcat(dst, src, size)
#endif
#else /* non-BSD platforms */
@@ -71,6 +85,7 @@ rte_strlcpy(char *dst, const char *src, size_t size)
#else /* no BSD header files, create own */
#define strlcpy(dst, src, size) rte_strlcpy(dst, src, size)
+#define strlcat(dst, src, size) rte_strlcat(dst, src, size)
#endif /* RTE_USE_LIBBSD */
#endif /* BSDAPP */
@@ -129,11 +129,56 @@ test_rte_strsplit(void)
return 0;
}
+int
+test_rte_strlcat(void)
+{
+ /* only run actual unit tests if we have system-provided strlcat */
+#if defined(__BSD_VISIBLE) || defined(RTE_USE_LIBBSD)
+#define BUF_LEN 32
+ const char dst[BUF_LEN] = "Test string";
+ const char src[] = " appended";
+ char bsd_dst[BUF_LEN];
+ char rte_dst[BUF_LEN];
+ size_t i, bsd_ret, rte_ret;
+
+ LOG("dst = '%s', strlen(dst) = %zu\n", dst, strlen(dst));
+ LOG("src = '%s', strlen(src) = %zu\n", src, strlen(src));
+ LOG("---\n");
+
+ for (i = 0; i < BUF_LEN; i++) {
+ /* initialize destination buffers */
+ memcpy(bsd_dst, dst, BUF_LEN);
+ memcpy(rte_dst, dst, BUF_LEN);
+ /* compare implementations */
+ bsd_ret = strlcat(bsd_dst, src, i);
+ rte_ret = rte_strlcat(rte_dst, src, i);
+ if (bsd_ret != rte_ret) {
+ LOG("Incorrect retval for buf length = %zu\n", i);
+ LOG("BSD: '%zu', rte: '%zu'\n", bsd_ret, rte_ret);
+ return -1;
+ }
+ if (memcmp(bsd_dst, rte_dst, BUF_LEN) != 0) {
+ LOG("Resulting buffers don't match\n");
+ LOG("BSD: '%s', rte: '%s'\n", bsd_dst, rte_dst);
+ return -1;
+ }
+ LOG("buffer size = %zu: dst = '%s', ret = %zu\n",
+ i, rte_dst, rte_ret);
+ }
+ LOG("Checked %zu combinations\n", i);
+#undef BUF_LEN
+#endif /* defined(__BSD_VISIBLE) || defined(RTE_USE_LIBBSD) */
+
+ return 0;
+}
+
static int
test_string_fns(void)
{
if (test_rte_strsplit() < 0)
return -1;
+ if (test_rte_strlcat() < 0)
+ return -1;
return 0;
}