* [dpdk-dev] strscpy() support to DPDK?
@ 2018-09-11 5:33 Kuusisaari, Juhamatti (Coriant - FI/Espoo)
2018-09-11 10:04 ` [dpdk-dev] [PATCH v1] eal: add strscpy function Gaetan Rivet
0 siblings, 1 reply; 6+ messages in thread
From: Kuusisaari, Juhamatti (Coriant - FI/Espoo) @ 2018-09-11 5:33 UTC (permalink / raw)
To: dev
Hello,
There has been some discussion earlier about the downsides of strlcpy().
How about adding strscpy() support to DPDK as a better option? It seems that it really resolves the API issues of strlcpy().
https://lwn.net/Articles/659214/
BR,
--
Juhamatti
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v1] eal: add strscpy function
2018-09-11 5:33 [dpdk-dev] strscpy() support to DPDK? Kuusisaari, Juhamatti (Coriant - FI/Espoo)
@ 2018-09-11 10:04 ` Gaetan Rivet
2018-09-11 10:17 ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
0 siblings, 1 reply; 6+ messages in thread
From: Gaetan Rivet @ 2018-09-11 10:04 UTC (permalink / raw)
To: dev; +Cc: Gaetan Rivet
The strncpy function has long been deemed unsafe for use,
in favor of strlcpy or snprintf.
While snprintf is standard and strlcpy is still largely available,
they both have issues regarding error checking and performance.
Both will force reading the source buffer past the requested size
if the input is not a proper c-string, and will return the expected
number of bytes copied, meaning that error checking needs to verify
that the number of bytes copied is not superior to the destination
size.
This contributes to awkward code flow, unclear error checking and
potential issues with malformed input.
The function strscpy has been discussed for some time already and
has been made available in the linux kernel[1].
Propose this new function as a safe alternative.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=30c44659f4a3e7e1f9f47e895591b4b40bf62671
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
I agree with the original email, here is a proposed implementation.
I have added the function as part of 18.11 API proper, because this API
is definitely not meant to change.
This is not meant to be enforced on existing code, or even on new code.
But I think it is better to have it available.
lib/librte_eal/common/eal_common_string_fns.c | 30 +++++++++++++++++++
.../common/include/rte_string_fns.h | 23 ++++++++++++++
lib/librte_eal/rte_eal_version.map | 7 +++++
3 files changed, 60 insertions(+)
diff --git a/lib/librte_eal/common/eal_common_string_fns.c b/lib/librte_eal/common/eal_common_string_fns.c
index 6ac5f8289..8a34d2422 100644
--- a/lib/librte_eal/common/eal_common_string_fns.c
+++ b/lib/librte_eal/common/eal_common_string_fns.c
@@ -38,3 +38,33 @@ rte_strsplit(char *string, int stringlen,
errno = EINVAL;
return -1;
}
+
+/* Copy src string into dst.
+ *
+ * Return negative value and NUL-terminate if dst is too short,
+ * Otherwise return number of bytes copied.
+ */
+ssize_t
+strscpy(char *dst, const char *src, size_t dsize)
+{
+ const char *osrc = src;
+ size_t nleft = dsize;
+
+ /* Copy as many bytes as will fit. */
+ if (nleft != 0) {
+ while (--nleft != 0) {
+ if ((*dst++ = *src++) == '\0')
+ break;
+ }
+ }
+
+ /* Not enough room in dst, add NUL and return error. */
+ if (nleft == 0) {
+ if (dsize != 0)
+ *dst = '\0';
+ return -E2BIG;
+ }
+
+ /* count does not include NUL */
+ return (src - osrc - 1);
+}
diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
index 97597a148..46dd919b4 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -76,6 +76,29 @@ rte_strlcpy(char *dst, const char *src, size_t size)
#endif /* RTE_USE_LIBBSD */
#endif /* BSDAPP */
+/**
+ * Copy string src to buffer dst of size dsize.
+ * At most dsize-1 chars will be copied.
+ * Always NUL-terminates, unless (dsize == 0).
+ * Returns number of bytes copied (terminating NUL-byte excluded) on success.
+ * Negative errno on error.
+ *
+ * @param dst
+ * The destination string.
+ *
+ * @param src
+ * The input string to be copied.
+ *
+ * @param dsize
+ * Length in bytes of the destination buffer.
+ *
+ * @return
+ * The number of bytes copied on success
+ * -E2BIG if the destination buffer is too small.
+ */
+ssize_t
+strscpy(char *dst, const char *src, size_t dsize);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 344a43d32..fc7b50669 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -262,6 +262,13 @@ DPDK_18.08 {
} DPDK_18.05;
+DPDK_18.11 {
+ global:
+
+ strscpy;
+
+} DPDK_18.08;
+
EXPERIMENTAL {
global:
--
2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v1] eal: add strscpy function
2018-09-11 10:04 ` [dpdk-dev] [PATCH v1] eal: add strscpy function Gaetan Rivet
@ 2018-09-11 10:17 ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
2018-09-11 15:00 ` [dpdk-dev] [PATCH v2] " Gaetan Rivet
0 siblings, 1 reply; 6+ messages in thread
From: Kuusisaari, Juhamatti (Coriant - FI/Espoo) @ 2018-09-11 10:17 UTC (permalink / raw)
To: Gaetan Rivet, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gaetan Rivet
> Sent: Tuesday, September 11, 2018 1:04 PM
> To: dev@dpdk.org
> Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
> Subject: [dpdk-dev] [PATCH v1] eal: add strscpy function
>
> The strncpy function has long been deemed unsafe for use,
> in favor of strlcpy or snprintf.
>
> While snprintf is standard and strlcpy is still largely available,
> they both have issues regarding error checking and performance.
>
> Both will force reading the source buffer past the requested size
> if the input is not a proper c-string, and will return the expected
> number of bytes copied, meaning that error checking needs to verify
> that the number of bytes copied is not superior to the destination
> size.
>
> This contributes to awkward code flow, unclear error checking and
> potential issues with malformed input.
>
> The function strscpy has been discussed for some time already and
> has been made available in the linux kernel[1].
>
> Propose this new function as a safe alternative.
>
> [1]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i
> d=30c44659f4a3e7e1f9f47e895591b4b40bf62671
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>
> I agree with the original email, here is a proposed implementation.
> I have added the function as part of 18.11 API proper, because this API
> is definitely not meant to change.
>
> This is not meant to be enforced on existing code, or even on new code.
> But I think it is better to have it available.
>
> lib/librte_eal/common/eal_common_string_fns.c | 30
> +++++++++++++++++++
> .../common/include/rte_string_fns.h | 23 ++++++++++++++
> lib/librte_eal/rte_eal_version.map | 7 +++++
> 3 files changed, 60 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_string_fns.c
> b/lib/librte_eal/common/eal_common_string_fns.c
> index 6ac5f8289..8a34d2422 100644
> --- a/lib/librte_eal/common/eal_common_string_fns.c
> +++ b/lib/librte_eal/common/eal_common_string_fns.c
> @@ -38,3 +38,33 @@ rte_strsplit(char *string, int stringlen,
> errno = EINVAL;
> return -1;
> }
> +
> +/* Copy src string into dst.
> + *
> + * Return negative value and NUL-terminate if dst is too short,
> + * Otherwise return number of bytes copied.
> + */
> +ssize_t
> +strscpy(char *dst, const char *src, size_t dsize)
> +{
> + const char *osrc = src;
> + size_t nleft = dsize;
> +
> + /* Copy as many bytes as will fit. */
> + if (nleft != 0) {
> + while (--nleft != 0) {
> + if ((*dst++ = *src++) == '\0')
> + break;
> + }
> + }
> +
> + /* Not enough room in dst, add NUL and return error. */
> + if (nleft == 0) {
> + if (dsize != 0)
> + *dst = '\0';
> + return -E2BIG;
> + }
> +
> + /* count does not include NUL */
> + return (src - osrc - 1);
> +}
> diff --git a/lib/librte_eal/common/include/rte_string_fns.h
> b/lib/librte_eal/common/include/rte_string_fns.h
> index 97597a148..46dd919b4 100644
> --- a/lib/librte_eal/common/include/rte_string_fns.h
> +++ b/lib/librte_eal/common/include/rte_string_fns.h
> @@ -76,6 +76,29 @@ rte_strlcpy(char *dst, const char *src, size_t size)
> #endif /* RTE_USE_LIBBSD */
> #endif /* BSDAPP */
>
> +/**
> + * Copy string src to buffer dst of size dsize.
> + * At most dsize-1 chars will be copied.
> + * Always NUL-terminates, unless (dsize == 0).
> + * Returns number of bytes copied (terminating NUL-byte excluded) on
> success.
> + * Negative errno on error.
> + *
> + * @param dst
> + * The destination string.
> + *
> + * @param src
> + * The input string to be copied.
> + *
> + * @param dsize
> + * Length in bytes of the destination buffer.
> + *
> + * @return
> + * The number of bytes copied on success
> + * -E2BIG if the destination buffer is too small.
> + */
> +ssize_t
> +strscpy(char *dst, const char *src, size_t dsize);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/librte_eal/rte_eal_version.map
> b/lib/librte_eal/rte_eal_version.map
> index 344a43d32..fc7b50669 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -262,6 +262,13 @@ DPDK_18.08 {
>
> } DPDK_18.05;
>
> +DPDK_18.11 {
> + global:
> +
> + strscpy;
> +
> +} DPDK_18.08;
> +
> EXPERIMENTAL {
> global:
Acked-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
> --
> 2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH v2] eal: add strscpy function
2018-09-11 10:17 ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
@ 2018-09-11 15:00 ` Gaetan Rivet
2018-09-12 13:29 ` Ferruh Yigit
0 siblings, 1 reply; 6+ messages in thread
From: Gaetan Rivet @ 2018-09-11 15:00 UTC (permalink / raw)
To: dev; +Cc: Gaetan Rivet
The strncpy function has long been deemed unsafe for use,
in favor of strlcpy or snprintf.
While snprintf is standard and strlcpy is still largely available,
they both have issues regarding error checking and performance.
Both will force reading the source buffer past the requested size
if the input is not a proper c-string, and will return the expected
number of bytes copied, meaning that error checking needs to verify
that the number of bytes copied is not superior to the destination
size.
This contributes to awkward code flow, unclear error checking and
potential issues with malformed input.
The function strscpy has been discussed for some time already and
has been made available in the linux kernel[1].
Propose this new function as a safe alternative.
[1]: http://git.kernel.org/linus/30c44659f4a3
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Acked-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
---
v2: use rte_prefix,
avoid assignment in if()
I agree with the original email, here is a proposed implementation.
I have added the function as part of 18.11 API proper, because this API
is definitely not meant to change.
This is not meant to be enforced on existing code, or even on new code.
But I think it is better to have it available.
lib/librte_eal/common/eal_common_string_fns.c | 26 +++++++++++++++++++
.../common/include/rte_string_fns.h | 23 ++++++++++++++++
lib/librte_eal/rte_eal_version.map | 7 +++++
3 files changed, 56 insertions(+)
diff --git a/lib/librte_eal/common/eal_common_string_fns.c b/lib/librte_eal/common/eal_common_string_fns.c
index 6ac5f8289..60c5dd66f 100644
--- a/lib/librte_eal/common/eal_common_string_fns.c
+++ b/lib/librte_eal/common/eal_common_string_fns.c
@@ -38,3 +38,29 @@ rte_strsplit(char *string, int stringlen,
errno = EINVAL;
return -1;
}
+
+/* Copy src string into dst.
+ *
+ * Return negative value and NUL-terminate if dst is too short,
+ * Otherwise return number of bytes copied.
+ */
+ssize_t
+rte_strscpy(char *dst, const char *src, size_t dsize)
+{
+ size_t nleft = dsize;
+ size_t res = 0;
+
+ /* Copy as many bytes as will fit. */
+ while (nleft != 0) {
+ dst[res] = src[res];
+ if (src[res] == '\0')
+ return res;
+ res++;
+ nleft--;
+ }
+
+ /* Not enough room in dst, set NUL and return error. */
+ if (res != 0)
+ dst[res - 1] = '\0';
+ return -E2BIG;
+}
diff --git a/lib/librte_eal/common/include/rte_string_fns.h b/lib/librte_eal/common/include/rte_string_fns.h
index 97597a148..ecd141b85 100644
--- a/lib/librte_eal/common/include/rte_string_fns.h
+++ b/lib/librte_eal/common/include/rte_string_fns.h
@@ -76,6 +76,29 @@ rte_strlcpy(char *dst, const char *src, size_t size)
#endif /* RTE_USE_LIBBSD */
#endif /* BSDAPP */
+/**
+ * Copy string src to buffer dst of size dsize.
+ * At most dsize-1 chars will be copied.
+ * Always NUL-terminates, unless (dsize == 0).
+ * Returns number of bytes copied (terminating NUL-byte excluded) on success ;
+ * negative errno on error.
+ *
+ * @param dst
+ * The destination string.
+ *
+ * @param src
+ * The input string to be copied.
+ *
+ * @param dsize
+ * Length in bytes of the destination buffer.
+ *
+ * @return
+ * The number of bytes copied on success
+ * -E2BIG if the destination buffer is too small.
+ */
+ssize_t
+rte_strscpy(char *dst, const char *src, size_t dsize);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 344a43d32..2031d7b15 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -262,6 +262,13 @@ DPDK_18.08 {
} DPDK_18.05;
+DPDK_18.11 {
+ global:
+
+ rte_strscpy;
+
+} DPDK_18.08;
+
EXPERIMENTAL {
global:
--
2.18.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal: add strscpy function
2018-09-11 15:00 ` [dpdk-dev] [PATCH v2] " Gaetan Rivet
@ 2018-09-12 13:29 ` Ferruh Yigit
2018-09-19 9:41 ` Thomas Monjalon
0 siblings, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2018-09-12 13:29 UTC (permalink / raw)
To: Gaetan Rivet, dev
On 9/11/2018 4:00 PM, Gaetan Rivet wrote:
> The strncpy function has long been deemed unsafe for use,
> in favor of strlcpy or snprintf.
>
> While snprintf is standard and strlcpy is still largely available,
> they both have issues regarding error checking and performance.
>
> Both will force reading the source buffer past the requested size
> if the input is not a proper c-string, and will return the expected
> number of bytes copied, meaning that error checking needs to verify
> that the number of bytes copied is not superior to the destination
> size.
>
> This contributes to awkward code flow, unclear error checking and
> potential issues with malformed input.
>
> The function strscpy has been discussed for some time already and
> has been made available in the linux kernel[1].
>
> Propose this new function as a safe alternative.
>
> [1]: http://git.kernel.org/linus/30c44659f4a3
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> Acked-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal: add strscpy function
2018-09-12 13:29 ` Ferruh Yigit
@ 2018-09-19 9:41 ` Thomas Monjalon
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2018-09-19 9:41 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev, Ferruh Yigit
12/09/2018 15:29, Ferruh Yigit:
> On 9/11/2018 4:00 PM, Gaetan Rivet wrote:
> > The strncpy function has long been deemed unsafe for use,
> > in favor of strlcpy or snprintf.
> >
> > While snprintf is standard and strlcpy is still largely available,
> > they both have issues regarding error checking and performance.
> >
> > Both will force reading the source buffer past the requested size
> > if the input is not a proper c-string, and will return the expected
> > number of bytes copied, meaning that error checking needs to verify
> > that the number of bytes copied is not superior to the destination
> > size.
> >
> > This contributes to awkward code flow, unclear error checking and
> > potential issues with malformed input.
> >
> > The function strscpy has been discussed for some time already and
> > has been made available in the linux kernel[1].
> >
> > Propose this new function as a safe alternative.
> >
> > [1]: http://git.kernel.org/linus/30c44659f4a3
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > Acked-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied, thanks
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-19 9:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-11 5:33 [dpdk-dev] strscpy() support to DPDK? Kuusisaari, Juhamatti (Coriant - FI/Espoo)
2018-09-11 10:04 ` [dpdk-dev] [PATCH v1] eal: add strscpy function Gaetan Rivet
2018-09-11 10:17 ` Kuusisaari, Juhamatti (Coriant - FI/Espoo)
2018-09-11 15:00 ` [dpdk-dev] [PATCH v2] " Gaetan Rivet
2018-09-12 13:29 ` Ferruh Yigit
2018-09-19 9:41 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).