[2/3] eal: uninline rte_str_to_size
Checks
Commit Message
There is no reason for rte_str_to_size() to be inline.
Move the implementation out of <rte_common.h>.
Export it as a stable ABI because it always has been public.
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
Now <rte_common.h> doesn't need to #include <ctypes.h> and <stdlib.h>,
but removing them breaks some DPDK code, may break user code too.
I'm not sure what is the compatibility policy in this regard.
If such a breakage is allowed, I'd remove includes and fix DPDK code.
lib/eal/common/eal_common_string_fns.c | 32 ++++++++++++++++++++++++++
lib/eal/include/rte_common.h | 30 ++----------------------
lib/eal/version.map | 1 +
3 files changed, 35 insertions(+), 28 deletions(-)
Comments
> From: Dmitry Kozlyuk [mailto:dmitry.kozliuk@gmail.com]
> Sent: Sunday, 21 August 2022 22.50
> To: dev@dpdk.org
> Cc: Dmitry Kozlyuk; Ray Kinsella
> Subject: [PATCH 2/3] eal: uninline rte_str_to_size
>
> There is no reason for rte_str_to_size() to be inline.
> Move the implementation out of <rte_common.h>.
> Export it as a stable ABI because it always has been public.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
> ---
> Now <rte_common.h> doesn't need to #include <ctypes.h> and <stdlib.h>,
> but removing them breaks some DPDK code, may break user code too.
> I'm not sure what is the compatibility policy in this regard.
> If such a breakage is allowed, I'd remove includes and fix DPDK code.
>
The question I'm asking myself here is: Do we want rte_common.h to include common headers like these, just so we don't need to include them elsewhere? I think not.
I'm in favor of the principle of keeping it clean: Remove them from rte_common.h, and deal with the consequences.
If we keep them, we will forget why they are there, and some day in the future, someone will ask what these unused headers are doing in <rte_common.h>.
On Mon, Aug 22, 2022 at 09:24:47AM +0200, Morten Brørup wrote:
> > From: Dmitry Kozlyuk [mailto:dmitry.kozliuk@gmail.com]
> > Sent: Sunday, 21 August 2022 22.50
> > To: dev@dpdk.org
> > Cc: Dmitry Kozlyuk; Ray Kinsella
> > Subject: [PATCH 2/3] eal: uninline rte_str_to_size
> >
> > There is no reason for rte_str_to_size() to be inline.
> > Move the implementation out of <rte_common.h>.
> > Export it as a stable ABI because it always has been public.
> >
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
>
> > ---
> > Now <rte_common.h> doesn't need to #include <ctypes.h> and <stdlib.h>,
> > but removing them breaks some DPDK code, may break user code too.
> > I'm not sure what is the compatibility policy in this regard.
> > If such a breakage is allowed, I'd remove includes and fix DPDK code.
> >
>
> The question I'm asking myself here is: Do we want rte_common.h to include common headers like these, just so we don't need to include them elsewhere? I think not.
>
> I'm in favor of the principle of keeping it clean: Remove them from rte_common.h, and deal with the consequences.
>
> If we keep them, we will forget why they are there, and some day in the future, someone will ask what these unused headers are doing in <rte_common.h>.
>
+1
Since removing headers is a build-time issue only and not runtime, I think
we should just remove them.
/Bruce
@@ -64,3 +64,35 @@ rte_strscpy(char *dst, const char *src, size_t dsize)
rte_errno = E2BIG;
return -rte_errno;
}
+
+uint64_t
+rte_str_to_size(const char *str)
+{
+ char *endptr;
+ unsigned long long size;
+
+ while (isspace((int)*str))
+ str++;
+ if (*str == '-')
+ return 0;
+
+ errno = 0;
+ size = strtoull(str, &endptr, 0);
+ if (errno)
+ return 0;
+
+ if (*endptr == ' ')
+ endptr++; /* allow 1 space gap */
+
+ switch (*endptr) {
+ case 'G': case 'g':
+ size *= 1024; /* fall-through */
+ case 'M': case 'm':
+ size *= 1024; /* fall-through */
+ case 'K': case 'k':
+ size *= 1024; /* fall-through */
+ default:
+ break;
+ }
+ return size;
+}
@@ -875,34 +875,8 @@ rte_log2_u64(uint64_t v)
* @return
* Number.
*/
-static inline uint64_t
-rte_str_to_size(const char *str)
-{
- char *endptr;
- unsigned long long size;
-
- while (isspace((int)*str))
- str++;
- if (*str == '-')
- return 0;
-
- errno = 0;
- size = strtoull(str, &endptr, 0);
- if (errno)
- return 0;
-
- if (*endptr == ' ')
- endptr++; /* allow 1 space gap */
-
- switch (*endptr){
- case 'G': case 'g': size *= 1024; /* fall-through */
- case 'M': case 'm': size *= 1024; /* fall-through */
- case 'K': case 'k': size *= 1024; /* fall-through */
- default:
- break;
- }
- return size;
-}
+uint64_t
+rte_str_to_size(const char *str);
/**
* Function to terminate the application immediately, printing an error
@@ -261,6 +261,7 @@ DPDK_23 {
rte_socket_id;
rte_socket_id_by_idx;
rte_srand;
+ rte_str_to_size;
rte_strerror;
rte_strscpy;
rte_strsplit;