[dpdk-dev,v2,1/7] cfgfile: remove EAL dependency
Checks
Commit Message
This patch removes the dependency to EAL in cfgfile library.
Signed-off-by: Jacek Piasecki <jacekx.piasecki@intel.com>
---
lib/librte_cfgfile/Makefile | 1 +
lib/librte_cfgfile/rte_cfgfile.c | 29 +++++++++++++++++------------
2 files changed, 18 insertions(+), 12 deletions(-)
Comments
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jacek Piasecki
> Sent: Monday, June 26, 2017 11:59 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Jain, Deepak K
> <deepak.k.jain@intel.com>; Piasecki, JacekX <jacekx.piasecki@intel.com>
> Subject: [dpdk-dev] [PATCH v2 1/7] cfgfile: remove EAL dependency
>
> This patch removes the dependency to EAL in cfgfile library.
>
> Signed-off-by: Jacek Piasecki <jacekx.piasecki@intel.com>
> ---
> lib/librte_cfgfile/Makefile | 1 +
> lib/librte_cfgfile/rte_cfgfile.c | 29 +++++++++++++++++------------
> 2 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
> index 755ef11..0bee43e 100644
> --- a/lib/librte_cfgfile/Makefile
> +++ b/lib/librte_cfgfile/Makefile
> @@ -38,6 +38,7 @@ LIB = librte_cfgfile.a
>
> CFLAGS += -O3
> CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
>
> EXPORT_MAP := rte_cfgfile_version.map
>
> diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
> index b54a523..c6ae3e3 100644
> --- a/lib/librte_cfgfile/rte_cfgfile.c
> +++ b/lib/librte_cfgfile/rte_cfgfile.c
> @@ -36,7 +36,6 @@
> #include <string.h>
> #include <ctype.h>
> #include <rte_common.h>
> -#include <rte_string_fns.h>
>
> #include "rte_cfgfile.h"
>
> @@ -258,19 +257,25 @@ rte_cfgfile_load_with_params(const char
> *filename, int flags,
>
> struct rte_cfgfile_section *sect =
> cfg->sections[curr_section];
> - int n;
> +
> char *split[2] = {NULL};
> - n = rte_strsplit(buffer, sizeof(buffer), split, 2, '=');
> - if (flags & CFG_FLAG_EMPTY_VALUES) {
> - if ((n < 1) || (n > 2)) {
> - printf("Error at line %d - cannot split
> string, n=%d\n",
> - lineno, n);
> - goto error1;
> - }
> + split[0] = buffer;
> + split[1] = memchr(buffer, '=', len);
> +
> + /* when delimeter not found */
> + if (split[1] == NULL) {
> + printf("Error at line %d - cannot "
> + "split string\n", lineno);
> + goto error1;
> } else {
> - if (n != 2) {
> - printf("Error at line %d - cannot split
> string, n=%d\n",
> - lineno, n);
> + /* when delimeter found */
> + *split[1] = '\0';
> + split[1]++;
> +
> + if (!(flags & CFG_FLAG_EMPTY_VALUES) &&
> + (*split[1] == '\0')) {
> + printf("Error at line %d - cannot "
> + "split string\n", lineno);
> goto error1;
> }
> }
> --
> 2.7.4
Please separate the cfg file functionality from the eal functionality into distinct patch sets.
New API for cfgfile library allows to create a cfgfile at runtime, add new
section, add entry in a section, update existing entry and save cfgfile
structure to INI file - opens up the possibility to have applications
dynamically build up a proper DPDK configuration, rather than
having to have a pre-existing one. Due the new API functions, simplification
of load() function was made. One new unit test to TEST app was added. It
contains an example of a large INI file whose parsing requires multiple
reallocation of memory.
---
v3:
split one patchset into two distinct patchsets:
1. cfgfile library and TEST app changes
2. EAL changes and examples (this patchset depends on cfgfile)
v2:
lib eal:
Rework of rte_eal_configure(struct rte_cfgfile *cfg, char *prgname).
Now this function load data from cfg structure and did initial
initialization of EAL arguments. Vdev argument are stored in different
subsections eg. DPDK.vdev0, DPDK.vdev1 etc. After execution of this
function it is necessary to call rte_eal_init to complete EAL
initialization. There is no more merging arguments from different
sources (cfg file and command line).
Added non_eal_configure to testpmd application.
Function maintain the same functionality as rte_eal_configure but
for non-eal arguments.
Added config JSON feature to testpmd last patch from patchset contain
example showing use of .json configuration files.
lib cfgfile:
Rework of add_section(), add_entry() new implementation
New members allocated_entries/sections, free_entries/sections
in rte_cfgfile structure, change in array of pointers
**sections, **entries instead of *sections[], *entries[]
Add set_entry() to update/overwrite already existing entry in cfgfile
struct
Add save() function to save on disc cfgfile structure in INI format
Rework of existing load() function simplifying the code
Add unit test realloc_sections() in TEST app for testing realloc/malloc
of new API functions, add test for save() function
Jacek Piasecki (3):
cfgfile: remove EAL dependency
cfgfile: add new functions to API
test/cfgfile: add new unit test
Kuba Kozak (1):
cfgfile: rework of load function
lib/librte_cfgfile/Makefile | 1 +
lib/librte_cfgfile/rte_cfgfile.c | 397 +++++++++++++++--------
lib/librte_cfgfile/rte_cfgfile.h | 76 +++++
lib/librte_cfgfile/rte_cfgfile_version.map | 11 +
test/test/test_cfgfile.c | 40 +++
test/test/test_cfgfiles/etc/realloc_sections.ini | 128 ++++++++
6 files changed, 514 insertions(+), 139 deletions(-)
create mode 100644 test/test/test_cfgfiles/etc/realloc_sections.ini
On Tue, Jun 27, 2017 at 12:26:46PM +0200, Jacek Piasecki wrote:
> New API for cfgfile library allows to create a cfgfile at runtime, add new
> section, add entry in a section, update existing entry and save cfgfile
> structure to INI file - opens up the possibility to have applications
> dynamically build up a proper DPDK configuration, rather than
> having to have a pre-existing one. Due the new API functions, simplification
> of load() function was made. One new unit test to TEST app was added. It
> contains an example of a large INI file whose parsing requires multiple
> reallocation of memory.
>
> ---
> v3:
> split one patchset into two distinct patchsets:
> 1. cfgfile library and TEST app changes
> 2. EAL changes and examples (this patchset depends on cfgfile)
> v2:
> lib eal:
> Rework of rte_eal_configure(struct rte_cfgfile *cfg, char *prgname).
> Now this function load data from cfg structure and did initial
> initialization of EAL arguments. Vdev argument are stored in different
> subsections eg. DPDK.vdev0, DPDK.vdev1 etc. After execution of this
> function it is necessary to call rte_eal_init to complete EAL
> initialization. There is no more merging arguments from different
> sources (cfg file and command line).
> Added non_eal_configure to testpmd application.
> Function maintain the same functionality as rte_eal_configure but
> for non-eal arguments.
> Added config JSON feature to testpmd last patch from patchset contain
> example showing use of .json configuration files.
>
> lib cfgfile:
> Rework of add_section(), add_entry() new implementation
> New members allocated_entries/sections, free_entries/sections
> in rte_cfgfile structure, change in array of pointers
> **sections, **entries instead of *sections[], *entries[]
> Add set_entry() to update/overwrite already existing entry in cfgfile
> struct
> Add save() function to save on disc cfgfile structure in INI format
> Rework of existing load() function simplifying the code
> Add unit test realloc_sections() in TEST app for testing realloc/malloc
> of new API functions, add test for save() function
>
> Jacek Piasecki (3):
> cfgfile: remove EAL dependency
> cfgfile: add new functions to API
> test/cfgfile: add new unit test
>
> Kuba Kozak (1):
> cfgfile: rework of load function
>
This looks a good set to have. Some cleanup work needed on the code, but
from a high-level implementation view, it's fine.
Once issues flagged by me are fixed:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
@@ -38,6 +38,7 @@ LIB = librte_cfgfile.a
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
EXPORT_MAP := rte_cfgfile_version.map
@@ -36,7 +36,6 @@
#include <string.h>
#include <ctype.h>
#include <rte_common.h>
-#include <rte_string_fns.h>
#include "rte_cfgfile.h"
@@ -258,19 +257,25 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
struct rte_cfgfile_section *sect =
cfg->sections[curr_section];
- int n;
+
char *split[2] = {NULL};
- n = rte_strsplit(buffer, sizeof(buffer), split, 2, '=');
- if (flags & CFG_FLAG_EMPTY_VALUES) {
- if ((n < 1) || (n > 2)) {
- printf("Error at line %d - cannot split string, n=%d\n",
- lineno, n);
- goto error1;
- }
+ split[0] = buffer;
+ split[1] = memchr(buffer, '=', len);
+
+ /* when delimeter not found */
+ if (split[1] == NULL) {
+ printf("Error at line %d - cannot "
+ "split string\n", lineno);
+ goto error1;
} else {
- if (n != 2) {
- printf("Error at line %d - cannot split string, n=%d\n",
- lineno, n);
+ /* when delimeter found */
+ *split[1] = '\0';
+ split[1]++;
+
+ if (!(flags & CFG_FLAG_EMPTY_VALUES) &&
+ (*split[1] == '\0')) {
+ printf("Error at line %d - cannot "
+ "split string\n", lineno);
goto error1;
}
}