[v3] eal: support strlcat function

Message ID 20190117173032.42076-1-bruce.richardson@intel.com
State Accepted
Delegated to: Thomas Monjalon
Headers show
Series
  • [v3] eal: support strlcat function
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Jan. 17, 2019, 5:30 p.m.
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>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

---
V3: fix compiler error by making the test function static
CC: Reshma Pattan <reshma.pattan@intel.com>

V2: place ternary operator with regular if statement for readability
CC: Ferruh Yigit <ferruh.yigit@intel.com>
---
 .../common/include/rte_string_fns.h           | 16 +++++++
 test/test/test_string_fns.c                   | 45 +++++++++++++++++++
 2 files changed, 61 insertions(+)

Comments

Thomas Monjalon Feb. 12, 2019, 9:27 a.m. | #1
17/01/2019 18:30, Bruce Richardson:
> 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>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied, thanks
Kevin Traynor April 16, 2019, 2:29 p.m. | #2
On 17/01/2019 17:30, 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>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Hi - I'd like to backport this to 18.11 stable branch. Not having it is
starting to cause build failures with backports. Let me know if any
objection.

thanks,
Kevin.
Bruce Richardson April 16, 2019, 2:37 p.m. | #3
On Tue, Apr 16, 2019 at 03:29:52PM +0100, Kevin Traynor wrote:
> On 17/01/2019 17:30, 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>
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Hi - I'd like to backport this to 18.11 stable branch. Not having it is
> starting to cause build failures with backports. Let me know if any
> objection.
> 
Go for it! No objections here, I think it makes sense.

/Bruce
Ferruh Yigit April 16, 2019, 4:03 p.m. | #4
On 4/16/2019 3:37 PM, Bruce Richardson wrote:
> On Tue, Apr 16, 2019 at 03:29:52PM +0100, Kevin Traynor wrote:
>> On 17/01/2019 17:30, 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>
>>> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> Hi - I'd like to backport this to 18.11 stable branch. Not having it is
>> starting to cause build failures with backports. Let me know if any
>> objection.
>>
> Go for it! No objections here, I think it makes sense.

+1 for the backport.

Patch

diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
index 9a2a1ff90..35c6b003c 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -59,10 +59,25 @@  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);
+	if (l < size)
+		return l + rte_strlcpy(&dst[l], src, size - l);
+	return 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 +86,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..5e105d2bb 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;
 }
 
+static 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;
 }