* [dpdk-dev] [PATCH] Don't inline string functions
@ 2014-05-14 15:19 Stephen Hemminger
2014-05-14 15:29 ` Richardson, Bruce
2014-05-14 15:47 ` Thomas Monjalon
0 siblings, 2 replies; 4+ messages in thread
From: Stephen Hemminger @ 2014-05-14 15:19 UTC (permalink / raw)
To: dev
It makes no sense to inline string functions, in fact snprintf
can't be inlined because the function supports variable number of
arguments.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
Resend because of lack of response
lib/librte_eal/bsdapp/eal/Makefile | 1
lib/librte_eal/common/eal_common_string_fns.c | 96 +++++++++++++++++++++++++
lib/librte_eal/common/include/rte_string_fns.h | 57 +-------------
lib/librte_eal/linuxapp/eal/Makefile | 1
4 files changed, 102 insertions(+), 53 deletions(-)
--- a/lib/librte_eal/bsdapp/eal/Makefile 2014-05-02 16:31:12.519570116 -0700
+++ b/lib/librte_eal/bsdapp/eal/Makefile 2014-05-02 16:31:12.519570116 -0700
@@ -67,6 +67,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) +=
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_tailqs.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_errno.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_cpuflags.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_string_fns.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_hexdump.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_devargs.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_vdev.c
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ b/lib/librte_eal/common/eal_common_string_fns.c 2014-05-02 16:31:12.519570116 -0700
@@ -0,0 +1,96 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include <stdarg.h>
+
+#include <rte_string_fns.h>
+
+/* safe version os snprintf */
+int
+rte_snprintf(char *buffer, int buflen, const char *format, ...)
+{
+ int len;
+ va_list ap;
+
+ if (buffer == NULL && buflen != 0)
+ goto einval_error;
+ if (format == NULL) {
+ if (buflen > 0)
+ buffer[0] = '\0';
+ goto einval_error;
+ }
+
+ va_start(ap, format);
+ len = vsnprintf(buffer, buflen, format, ap);
+ va_end(ap);
+ if (len >= buflen && buflen > 0)
+ buffer[buflen - 1] = '\0';
+
+ return len;
+
+einval_error:
+ errno = EINVAL;
+ return -1;
+}
+
+/* split string into tokens */
+int
+rte_strsplit(char *string, int stringlen,
+ char **tokens, int maxtokens, char delim)
+{
+ int i, tok = 0;
+ int tokstart = 1; /* first token is right at start of string */
+
+ if (string == NULL || tokens == NULL)
+ goto einval_error;
+
+ for (i = 0; i < stringlen; i++) {
+ if (string[i] == '\0' || tok >= maxtokens)
+ break;
+ if (tokstart) {
+ tokstart = 0;
+ tokens[tok++] = &string[i];
+ }
+ if (string[i] == delim) {
+ string[i] = '\0';
+ tokstart = 1;
+ }
+ }
+ return tok;
+
+einval_error:
+ errno = EINVAL;
+ return -1;
+}
--- a/lib/librte_eal/common/include/rte_string_fns.h 2014-05-02 16:31:12.519570116 -0700
+++ b/lib/librte_eal/common/include/rte_string_fns.h 2014-05-02 16:31:12.519570116 -0700
@@ -72,33 +72,9 @@ extern "C" {
* buffer been sufficiently big.
*
*/
-static inline int
-rte_snprintf(char *buffer, int buflen, const char *format, ...)
-{
- int len;
- va_list ap;
-
- if (buffer == NULL && buflen != 0)
- goto einval_error;
- if (format == NULL) {
- if (buflen > 0)
- buffer[0] = '\0';
- goto einval_error;
- }
-
- va_start(ap, format);
- len = vsnprintf(buffer, buflen, format, ap);
- va_end(ap);
- if (len >= buflen && buflen > 0)
- buffer[buflen - 1] = '\0';
-
- return len;
-
-einval_error:
- errno = EINVAL;
- return -1;
-}
+int
+rte_snprintf(char *buffer, int buflen, const char *format, ...);
/**
* Takes string "string" parameter and splits it at character "delim"
@@ -126,34 +102,9 @@ einval_error:
* @return
* The number of tokens in the tokens array.
*/
-static inline int
+int
rte_strsplit(char *string, int stringlen,
- char **tokens, int maxtokens, char delim)
-{
- int i, tok = 0;
- int tokstart = 1; /* first token is right at start of string */
-
- if (string == NULL || tokens == NULL)
- goto einval_error;
-
- for (i = 0; i < stringlen; i++) {
- if (string[i] == '\0' || tok >= maxtokens)
- break;
- if (tokstart) {
- tokstart = 0;
- tokens[tok++] = &string[i];
- }
- if (string[i] == delim) {
- string[i] = '\0';
- tokstart = 1;
- }
- }
- return tok;
-
-einval_error:
- errno = EINVAL;
- return -1;
-}
+ char **tokens, int maxtokens, char delim);
#ifdef __cplusplus
}
--- a/lib/librte_eal/linuxapp/eal/Makefile 2014-05-02 16:31:12.519570116 -0700
+++ b/lib/librte_eal/linuxapp/eal/Makefile 2014-05-02 16:31:12.519570116 -0700
@@ -75,6 +75,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) +
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_tailqs.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_errno.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_cpuflags.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_string_fns.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_hexdump.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_devargs.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_vdev.c
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] Don't inline string functions
2014-05-14 15:19 [dpdk-dev] [PATCH] Don't inline string functions Stephen Hemminger
@ 2014-05-14 15:29 ` Richardson, Bruce
2014-05-14 15:47 ` Thomas Monjalon
1 sibling, 0 replies; 4+ messages in thread
From: Richardson, Bruce @ 2014-05-14 15:29 UTC (permalink / raw)
To: Stephen Hemminger, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Wednesday, May 14, 2014 4:20 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] Don't inline string functions
>
> It makes no sense to inline string functions, in fact snprintf can't be inlined
> because the function supports variable number of arguments.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] Don't inline string functions
2014-05-14 15:19 [dpdk-dev] [PATCH] Don't inline string functions Stephen Hemminger
2014-05-14 15:29 ` Richardson, Bruce
@ 2014-05-14 15:47 ` Thomas Monjalon
2014-05-14 16:41 ` Stephen Hemminger
1 sibling, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2014-05-14 15:47 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev
2014-05-14 08:19, Stephen Hemminger:
> It makes no sense to inline string functions, in fact snprintf
> can't be inlined because the function supports variable number of
> arguments.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
> ---
> Resend because of lack of response
Please do not resend patches. I prefer a ping request in the original thread.
By the way, you didn't take into account my first comments:
http://dpdk.org/ml/archives/dev/2014-April/002028.html
and I'm still waiting answers to these patches:
http://dpdk.org/ml/archives/dev/2014-March/001673.html
http://dpdk.org/ml/archives/dev/2014-March/001676.html
PS: I was going to apply your patches with these fixes:
add FILE argument to debug functions
[Thomas: fix quota_watermark example]
eal: dont inline string functions
[Thomas: update includes]
--
Thomas
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-05-14 16:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-14 15:19 [dpdk-dev] [PATCH] Don't inline string functions Stephen Hemminger
2014-05-14 15:29 ` Richardson, Bruce
2014-05-14 15:47 ` Thomas Monjalon
2014-05-14 16:41 ` Stephen Hemminger
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).