[v3,1/7] eal: add wrappers for POSIX string functions
Checks
Commit Message
POSIX strncasecmp(), strdup(), and strtok_r() have different names
on Windows, respectively, strnicmp(), _strdup(), and strtok_s().
Add wrappers as inline functions, because they're used from librte_kvargs,
and thus cannot be in librte_eal; besides, implementation is trivial.
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
lib/librte_eal/common/eal_common_dev.c | 6 +--
lib/librte_eal/common/eal_common_devargs.c | 7 ++--
lib/librte_eal/common/eal_common_log.c | 5 ++-
lib/librte_eal/common/eal_common_options.c | 12 +++---
lib/librte_eal/common/eal_common_trace_ctf.c | 2 +-
.../common/eal_common_trace_utils.c | 2 +-
lib/librte_eal/include/rte_string_fns.h | 42 +++++++++++++++++++
7 files changed, 60 insertions(+), 16 deletions(-)
Comments
On 2/21/21 5:28 PM, Dmitry Kozlyuk wrote:
> POSIX strncasecmp(), strdup(), and strtok_r() have different names
> on Windows, respectively, strnicmp(), _strdup(), and strtok_s().
>
> Add wrappers as inline functions, because they're used from librte_kvargs,
> and thus cannot be in librte_eal; besides, implementation is trivial.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
[snip]
> +/**
> + * @internal
> + * strdup(3) replacement for systems that don't have it.
> + */
> +static inline char *
> +rte_strdup(const char *str)
> +{
> +#ifdef RTE_EXEC_ENV_WINDOWS
> + return _strdup(str);
> +#else
> + return strdup(str);
> +#endif
> +}
Allocating memory using rte_strdup() I'd use rte_free()
to release it. I guess it will fail badly.
So, I think that a different, more specific prefix is
required for POSIX wrappers.
> Allocating memory using rte_strdup() I'd use rte_free()
> to release it. I guess it will fail badly.
> So, I think that a different, more specific prefix is
> required for POSIX wrappers.
Andrew: my understanding of Bruce's proposal is that the strdup() name
will now be kept (in this case through an inline definition), so I think
this will be ok. However, your comment reminded me of something else
that it's probably worth mentioning as an aside:
As a general guideline on Windows, memory allocated within a shared
library is best freed within the same DLL to ensure it goes back to the
correct heap. So we'd want to avoid calling strdup and then returning
the value to the application for it to free (hopefully this doesn't
happen). With an inline definition there's no change in behaviour, but
adding rte_strdup (or anything else that calls malloc) into librte_eal
might be an issue.
Regards,
Nick
On 2/24/21 12:53 AM, Nick Connolly wrote:
>
>> Allocating memory using rte_strdup() I'd use rte_free()
>> to release it. I guess it will fail badly.
>> So, I think that a different, more specific prefix is
>> required for POSIX wrappers.
>
> Andrew: my understanding of Bruce's proposal is that the strdup() name
> will now be kept (in this case through an inline definition), so I think
> this will be ok.
Very good, glad to hear it. Thanks.
> However, your comment reminded me of something else
> that it's probably worth mentioning as an aside:
>
> As a general guideline on Windows, memory allocated within a shared
> library is best freed within the same DLL to ensure it goes back to the
> correct heap. So we'd want to avoid calling strdup and then returning
> the value to the application for it to free (hopefully this doesn't
> happen). With an inline definition there's no change in behaviour, but
> adding rte_strdup (or anything else that calls malloc) into librte_eal
> might be an issue.
>
> Regards,
> Nick
> POSIX strncasecmp(), strdup(), and strtok_r() have different names
> on Windows, respectively, strnicmp(), _strdup(), and strtok_s().
>
> Add wrappers as inline functions, because they're used from librte_kvargs,
> and thus cannot be in librte_eal; besides, implementation is trivial.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
Acked-by: Khoa To <khot@microsoft.com>
@@ -462,7 +462,7 @@ rte_dev_event_callback_register(const char *device_name,
if (!device_name) {
event_cb->dev_name = NULL;
} else {
- event_cb->dev_name = strdup(device_name);
+ event_cb->dev_name = rte_strdup(device_name);
if (event_cb->dev_name == NULL) {
ret = -ENOMEM;
goto error;
@@ -630,10 +630,10 @@ dev_str_sane_copy(const char *str)
end = strcspn(str, ",/");
if (str[end] == ',') {
- copy = strdup(&str[end + 1]);
+ copy = rte_strdup(&str[end + 1]);
} else {
/* '/' or '\0' */
- copy = strdup("");
+ copy = rte_strdup("");
}
if (copy == NULL) {
rte_errno = ENOMEM;
@@ -18,6 +18,7 @@
#include <rte_errno.h>
#include <rte_kvargs.h>
#include <rte_log.h>
+#include <rte_string_fns.h>
#include <rte_tailq.h>
#include "eal_private.h"
@@ -75,7 +76,7 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
* anything and keep referring only to it.
*/
if (devargs->data != devstr) {
- devargs->data = strdup(devstr);
+ devargs->data = rte_strdup(devstr);
if (devargs->data == NULL) {
RTE_LOG(ERR, EAL, "OOM\n");
ret = -ENOMEM;
@@ -219,9 +220,9 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)
da->bus = bus;
/* Parse eventual device arguments */
if (devname[i] == ',')
- da->args = strdup(&devname[i + 1]);
+ da->args = rte_strdup(&devname[i + 1]);
else
- da->args = strdup("");
+ da->args = rte_strdup("");
if (da->args == NULL) {
RTE_LOG(ERR, EAL, "not enough memory to parse arguments\n");
return -ENOMEM;
@@ -14,6 +14,7 @@
#include <rte_eal.h>
#include <rte_log.h>
#include <rte_per_lcore.h>
+#include <rte_string_fns.h>
#include "eal_private.h"
@@ -194,7 +195,7 @@ static int rte_log_save_level(int priority,
if (regcomp(&opt_ll->re_match, regex, 0) != 0)
goto fail;
} else if (pattern) {
- opt_ll->pattern = strdup(pattern);
+ opt_ll->pattern = rte_strdup(pattern);
if (opt_ll->pattern == NULL)
goto fail;
} else
@@ -270,7 +271,7 @@ rte_log_lookup(const char *name)
static int
__rte_log_register(const char *name, int id)
{
- char *dup_name = strdup(name);
+ char *dup_name = rte_strdup(name);
if (dup_name == NULL)
return -ENOMEM;
@@ -228,7 +228,7 @@ eal_save_args(int argc, char **argv)
return -1;
for (i = 0; i < argc; i++) {
- eal_args[i] = strdup(argv[i]);
+ eal_args[i] = rte_strdup(argv[i]);
if (strcmp(argv[i], "--") == 0)
break;
}
@@ -243,7 +243,7 @@ eal_save_args(int argc, char **argv)
return -1;
for (j = 0; i < argc; j++, i++)
- eal_app_args[j] = strdup(argv[i]);
+ eal_app_args[j] = rte_strdup(argv[i]);
eal_app_args[j] = NULL;
return 0;
@@ -1273,7 +1273,7 @@ eal_parse_log_level(const char *arg)
char *str, *level;
int priority;
- str = strdup(arg);
+ str = rte_strdup(arg);
if (str == NULL)
return -1;
@@ -1324,11 +1324,11 @@ eal_parse_log_level(const char *arg)
static enum rte_proc_type_t
eal_parse_proc_type(const char *arg)
{
- if (strncasecmp(arg, "primary", sizeof("primary")) == 0)
+ if (rte_strncasecmp(arg, "primary", sizeof("primary")) == 0)
return RTE_PROC_PRIMARY;
- if (strncasecmp(arg, "secondary", sizeof("secondary")) == 0)
+ if (rte_strncasecmp(arg, "secondary", sizeof("secondary")) == 0)
return RTE_PROC_SECONDARY;
- if (strncasecmp(arg, "auto", sizeof("auto")) == 0)
+ if (rte_strncasecmp(arg, "auto", sizeof("auto")) == 0)
return RTE_PROC_AUTO;
return RTE_PROC_INVALID;
@@ -398,7 +398,7 @@ char *trace_metadata_fixup_field(const char *field)
if (strstr(field, ".") == NULL && strstr(field, "->") == NULL)
return NULL;
- out = strdup(field);
+ out = rte_strdup(field);
if (out == NULL)
return NULL;
p = out;
@@ -145,7 +145,7 @@ eal_trace_args_save(const char *val)
return -ENOMEM;
}
- arg->val = strdup(val);
+ arg->val = rte_strdup(val);
if (arg->val == NULL) {
trace_err("failed to allocate memory for %s", val);
free(arg);
@@ -116,6 +116,48 @@ rte_strlcat(char *dst, const char *src, size_t size)
ssize_t
rte_strscpy(char *dst, const char *src, size_t dsize);
+/**
+ * @internal
+ * strncasecmp(3) replacement for systems that don't have it.
+ */
+static inline int
+rte_strncasecmp(const char *s1, const char *s2, size_t size)
+{
+#ifdef RTE_EXEC_ENV_WINDOWS
+ return _strnicmp(s1, s2, size);
+#else
+ return strncasecmp(s1, s2, size);
+#endif
+}
+
+/**
+ * @internal
+ * strtor_r(3) replacement for systems that don't have it.
+ */
+static inline char *
+rte_strtok(char *str, const char *delim, char **saveptr)
+{
+#ifdef RTE_EXEC_ENV_WINDOWS
+ return strtok_s(str, delim, saveptr);
+#else
+ return strtok_r(str, delim, saveptr);
+#endif
+}
+
+/**
+ * @internal
+ * strdup(3) replacement for systems that don't have it.
+ */
+static inline char *
+rte_strdup(const char *str)
+{
+#ifdef RTE_EXEC_ENV_WINDOWS
+ return _strdup(str);
+#else
+ return strdup(str);
+#endif
+}
+
#ifdef __cplusplus
}
#endif