summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
authorOlivier Matz <olivier.matz@6wind.com>2014-01-28 17:06:38 +0100
committerThomas Monjalon <thomas.monjalon@6wind.com>2014-02-04 15:40:41 +0100
commite9b85f462dcfd70531eb6af0daa8139a806bc3ec (patch)
tree4d0065ba614bf3a117aa1a2434f8d3d3872ef730 /lib
parent6b0bc51b8fe1785890bce4652b42ad285d647a21 (diff)
downloaddpdk-e9b85f462dcfd70531eb6af0daa8139a806bc3ec.zip
dpdk-e9b85f462dcfd70531eb6af0daa8139a806bc3ec.tar.gz
dpdk-e9b85f462dcfd70531eb6af0daa8139a806bc3ec.tar.xz
kvargs: rework API to fix memory leak
Before the patch, a call to rte_kvargs_tokenize() resulted in a call to strdup() to allocate a modifiable copy of the argument string. This string was never freed, excepted in the error cases of rte_kvargs_tokenize() where rte_free() was wrongly called instead of free(). In other cases, freeing this string was impossible as the pointer not saved. This patch introduces rte_kvargs_free() in order to free the structure properly. The pointer to the duplicated string is now kept in the rte_kvargs structure. A call to rte_kvargs_parse() directly allocates the structure, making rte_kvargs_init() useless. The only drawback of this API change is that a key/value associations cannot be added to an existing kvlist. But it's not used today, and there is not obvious use case for that. Signed-off-by: Olivier Matz <olivier.matz@6wind.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Diffstat (limited to 'lib')
-rw-r--r--lib/librte_kvargs/Makefile3
-rw-r--r--lib/librte_kvargs/rte_kvargs.c64
-rw-r--r--lib/librte_kvargs/rte_kvargs.h42
-rw-r--r--lib/librte_pmd_pcap/rte_eth_pcap.c27
4 files changed, 63 insertions, 73 deletions
diff --git a/lib/librte_kvargs/Makefile b/lib/librte_kvargs/Makefile
index 16f6107..b09359a 100644
--- a/lib/librte_kvargs/Makefile
+++ b/lib/librte_kvargs/Makefile
@@ -45,8 +45,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_KVARGS) := rte_kvargs.c
INCS := rte_kvargs.h
SYMLINK-$(CONFIG_RTE_LIBRTE_KVARGS)-include := $(INCS)
-# this lib needs eal and malloc
+# this lib needs eal
DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_eal
-DEPDIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += lib/librte_malloc
include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_kvargs/rte_kvargs.c b/lib/librte_kvargs/rte_kvargs.c
index b05a9bc..f368956 100644
--- a/lib/librte_kvargs/rte_kvargs.c
+++ b/lib/librte_kvargs/rte_kvargs.c
@@ -34,25 +34,14 @@
#include <string.h>
#include <sys/user.h>
#include <linux/binfmts.h>
+#include <stdlib.h>
-#include <rte_malloc.h>
#include <rte_log.h>
#include <rte_string_fns.h>
#include "rte_kvargs.h"
/*
- * Initialize a rte_kvargs structure to an empty key/value list.
- */
-int
-rte_kvargs_init(struct rte_kvargs *kvlist)
-{
- kvlist->count = 0;
- memset(kvlist->pairs, 0, sizeof(kvlist->pairs));
- return 0;
-}
-
-/*
* Add a key-value pair at the end of a given key/value list.
* Return an error if the list is full or if the key is duplicated.
*/
@@ -93,7 +82,6 @@ static int
rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
{
unsigned i, count;
- char *args;
char *pairs[RTE_KVARGS_MAX];
char *pair[2];
@@ -106,13 +94,13 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
/* Copy the const char *params to a modifiable string
* to pass to rte_strsplit
*/
- args = strdup(params);
- if(args == NULL){
+ kvlist->str = strdup(params);
+ if (kvlist->str == NULL) {
RTE_LOG(ERR, PMD, "Cannot parse arguments: not enough memory\n");
return -1;
}
- count = rte_strsplit(args, strnlen(args, MAX_ARG_STRLEN), pairs,
+ count = rte_strsplit(kvlist->str, strnlen(kvlist->str, MAX_ARG_STRLEN), pairs,
RTE_KVARGS_MAX, RTE_KVARGS_PAIRS_DELIM);
for (i = 0; i < count; i++) {
@@ -127,17 +115,13 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)
RTE_LOG(ERR, PMD,
"Cannot parse arguments: wrong key or value\n"
"params=<%s>\n", params);
- goto error;
+ return -1;
}
if (rte_kvargs_add_pair(kvlist, pair[0], pair[1]) < 0)
- goto error;
+ return -1;
}
return 0;
-
-error:
- rte_free(args);
- return -1;
}
/*
@@ -222,25 +206,39 @@ rte_kvargs_process(const struct rte_kvargs *kvlist,
return 0;
}
+/* free the rte_kvargs structure */
+void
+rte_kvargs_free(struct rte_kvargs *kvlist)
+{
+ if (kvlist->str != NULL)
+ free(kvlist->str);
+ free(kvlist);
+}
+
/*
* Parse the arguments "key=value;key=value;..." string and return
* an allocated structure that contains a key/value list. Also
* check if only valid keys were used.
*/
-int
-rte_kvargs_parse(struct rte_kvargs *kvlist,
- const char *args,
- const char *valid_keys[])
+struct rte_kvargs *
+rte_kvargs_parse(const char *args, const char *valid_keys[])
{
+ struct rte_kvargs *kvlist;
- int ret;
+ kvlist = malloc(sizeof(*kvlist));
+ if (kvlist == NULL)
+ return NULL;
+ memset(kvlist, 0, sizeof(*kvlist));
- ret = rte_kvargs_tokenize(kvlist, args);
- if (ret < 0)
- return ret;
+ if (rte_kvargs_tokenize(kvlist, args) < 0) {
+ rte_kvargs_free(kvlist);
+ return NULL;
+ }
- if (valid_keys == NULL)
- return 0;
+ if (valid_keys != NULL && check_for_valid_keys(kvlist, valid_keys) < 0) {
+ rte_kvargs_free(kvlist);
+ return NULL;
+ }
- return check_for_valid_keys(kvlist, valid_keys);
+ return kvlist;
}
diff --git a/lib/librte_kvargs/rte_kvargs.h b/lib/librte_kvargs/rte_kvargs.h
index 3540e03..25e086f 100644
--- a/lib/librte_kvargs/rte_kvargs.h
+++ b/lib/librte_kvargs/rte_kvargs.h
@@ -70,47 +70,41 @@ struct rte_kvargs_pair {
/** Store a list of key/value associations */
struct rte_kvargs {
+ char *str; /**< copy of the argument string */
unsigned count; /**< number of entries in the list */
struct rte_kvargs_pair pairs[RTE_KVARGS_MAX]; /**< list of key/values */
};
/**
- * Initialize an empty rte_kvargs structure
+ * Allocate a rte_kvargs and store key/value associations from a string
*
- * Set the content of the rte_kvargs structure given as a parameter
- * to an empty list of key/value.
+ * The function allocates and fills a rte_kvargs structure from a given
+ * string whose format is key1=value1;key2=value2;...
*
- * @param kvlist
- * The rte_kvargs structure
+ * The structure can be freed with rte_kvargs_free().
+ *
+ * @param args
+ * The input string containing the key/value associations
+ * @param valid_keys
+ * A list of valid keys (table of const char *, the last must be NULL).
+ * This argument is ignored if NULL
*
* @return
- * - 0 on success
- * - Negative on error
+ * - A pointer to an allocated rte_kvargs structure on success
+ * - NULL on error
*/
-int rte_kvargs_init(struct rte_kvargs *kvlist);
+struct rte_kvargs *rte_kvargs_parse(const char *args, const char *valid_keys[]);
/**
- * Append key/value associations in a rte_kvargs structure from a string
+ * Free a rte_kvargs structure
*
- * The function fills a rte_kvargs structure from a string whose format
- * is key1=value1;key2=value2;...
- * The key/value associations are appended to those which are already
- * present in the rte_kvargs structure.
+ * Free a rte_kvargs structure previously allocated with
+ * rte_kvargs_parse().
*
* @param kvlist
* The rte_kvargs structure
- * @param args
- * The input string containing the key/value associations
- * @param valid_keys
- * A list of valid keys (table of const char *, the last must be NULL).
- * This argument is ignored if NULL
- *
- * @return
- * - 0 on success
- * - Negative on error
*/
-int rte_kvargs_parse(struct rte_kvargs *kvlist,
- const char *args, const char *valid_keys[]);
+void rte_kvargs_free(struct rte_kvargs *kvlist);
/**
* Call a handler function for each key/value matching the key
diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/rte_eth_pcap.c
index 2006b35..1f46cc9 100644
--- a/lib/librte_pmd_pcap/rte_eth_pcap.c
+++ b/lib/librte_pmd_pcap/rte_eth_pcap.c
@@ -702,30 +702,29 @@ rte_pmd_pcap_init(const char *name, const char *params)
{
unsigned numa_node, using_dumpers = 0;
int ret;
- struct rte_kvargs kvlist;
+ struct rte_kvargs *kvlist;
struct rx_pcaps pcaps;
struct tx_pcaps dumpers;
RTE_LOG(INFO, PMD, "Initializing pmd_pcap for %s\n", name);
- rte_kvargs_init(&kvlist);
-
numa_node = rte_socket_id();
gettimeofday(&start_time, NULL);
start_cycles = rte_get_timer_cycles();
hz = rte_get_timer_hz();
- if (rte_kvargs_parse(&kvlist, params, valid_arguments) < 0)
+ kvlist = rte_kvargs_parse(params, valid_arguments);
+ if (kvlist == NULL)
return -1;
/*
* If iface argument is passed we open the NICs and use them for
* reading / writing
*/
- if (rte_kvargs_count(&kvlist, ETH_PCAP_IFACE_ARG) == 1) {
+ if (rte_kvargs_count(kvlist, ETH_PCAP_IFACE_ARG) == 1) {
- ret = rte_kvargs_process(&kvlist, ETH_PCAP_IFACE_ARG,
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG,
&open_rx_tx_iface, &pcaps.pcaps[0]);
if (ret < 0)
return -1;
@@ -737,13 +736,13 @@ rte_pmd_pcap_init(const char *name, const char *params)
* We check whether we want to open a RX stream from a real NIC or a
* pcap file
*/
- if ((pcaps.num_of_rx = rte_kvargs_count(&kvlist, ETH_PCAP_RX_PCAP_ARG))) {
- ret = rte_kvargs_process(&kvlist, ETH_PCAP_RX_PCAP_ARG,
+ if ((pcaps.num_of_rx = rte_kvargs_count(kvlist, ETH_PCAP_RX_PCAP_ARG))) {
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_PCAP_ARG,
&open_rx_pcap, &pcaps);
} else {
- pcaps.num_of_rx = rte_kvargs_count(&kvlist,
+ pcaps.num_of_rx = rte_kvargs_count(kvlist,
ETH_PCAP_RX_IFACE_ARG);
- ret = rte_kvargs_process(&kvlist, ETH_PCAP_RX_IFACE_ARG,
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_RX_IFACE_ARG,
&open_rx_iface, &pcaps);
}
@@ -754,15 +753,15 @@ rte_pmd_pcap_init(const char *name, const char *params)
* We check whether we want to open a TX stream to a real NIC or a
* pcap file
*/
- if ((dumpers.num_of_tx = rte_kvargs_count(&kvlist,
+ if ((dumpers.num_of_tx = rte_kvargs_count(kvlist,
ETH_PCAP_TX_PCAP_ARG))) {
- ret = rte_kvargs_process(&kvlist, ETH_PCAP_TX_PCAP_ARG,
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_PCAP_ARG,
&open_tx_pcap, &dumpers);
using_dumpers = 1;
} else {
- dumpers.num_of_tx = rte_kvargs_count(&kvlist,
+ dumpers.num_of_tx = rte_kvargs_count(kvlist,
ETH_PCAP_TX_IFACE_ARG);
- ret = rte_kvargs_process(&kvlist, ETH_PCAP_TX_IFACE_ARG,
+ ret = rte_kvargs_process(kvlist, ETH_PCAP_TX_IFACE_ARG,
&open_tx_iface, &dumpers);
}