DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches
@ 2019-07-16 17:27 Stephen Hemminger
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-16 17:27 UTC (permalink / raw)
  To: cristian.dumitrescu; +Cc: dev, Stephen Hemminger

Some small patches for the cfgfile library

Stephen Hemminger (3):
  cfgfile: remove unnecessary initialization
  cfgfile: use RTE_LOG for errors
  cfgfile: use calloc

 lib/librte_cfgfile/rte_cfgfile.c        | 42 ++++++++++++++-----------
 lib/librte_eal/common/include/rte_log.h |  1 +
 2 files changed, 24 insertions(+), 19 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH 1/3] cfgfile: remove unnecessary initialization
  2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
@ 2019-07-16 17:27 ` Stephen Hemminger
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-16 17:27 UTC (permalink / raw)
  To: cristian.dumitrescu; +Cc: dev, Stephen Hemminger

No need to initialize variable if it is immediately overwritten.
It is better style not do unnecessary initialization with modern
tools since it lets compiler and other static checkers detect
uninitialized data.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index f8e7627a5169..1ef298592fa5 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -160,9 +160,9 @@ struct rte_cfgfile *
 rte_cfgfile_load_with_params(const char *filename, int flags,
 			     const struct rte_cfgfile_parameters *params)
 {
-	char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4] = {0};
+	char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4];
 	int lineno = 0;
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 
 	if (rte_cfgfile_check_params(params))
 		return NULL;
@@ -174,7 +174,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 	cfg = rte_cfgfile_create(flags);
 
 	while (fgets(buffer, sizeof(buffer), f) != NULL) {
-		char *pos = NULL;
+		char *pos;
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
@@ -260,7 +260,7 @@ struct rte_cfgfile *
 rte_cfgfile_create(int flags)
 {
 	int i;
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 
 	cfg = malloc(sizeof(*cfg));
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors
  2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
@ 2019-07-16 17:27 ` Stephen Hemminger
  2019-07-17 21:01   ` Thomas Monjalon
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 3/3] cfgfile: use calloc Stephen Hemminger
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-16 17:27 UTC (permalink / raw)
  To: cristian.dumitrescu; +Cc: dev, Stephen Hemminger

In general, DPDK libraries to not print error messages to
stdout because that is often redirected to /dev/null for daemons.
This patch changes cfgfile library to use RTE_LOG with its
own type.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c        | 25 +++++++++++++++----------
 lib/librte_eal/common/include/rte_log.h |  1 +
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 1ef298592fa5..c4b768b6833f 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <rte_string_fns.h>
 #include <rte_common.h>
+#include <rte_log.h>
 
 #include "rte_cfgfile.h"
 
@@ -128,7 +129,7 @@ rte_cfgfile_check_params(const struct rte_cfgfile_parameters *params)
 	unsigned int i;
 
 	if (!params) {
-		printf("Error - missing cfgfile parameters\n");
+		RTE_LOG(ERR, CFGFILE, "missing cfgfile parameters\n");
 		return -EINVAL;
 	}
 
@@ -141,7 +142,7 @@ rte_cfgfile_check_params(const struct rte_cfgfile_parameters *params)
 	}
 
 	if (valid_comment == 0)	{
-		printf("Error - invalid comment characters %c\n",
+		RTE_LOG(ERR, CFGFILE, "invalid comment characters %c\n",
 		       params->comment_character);
 		return -ENOTSUP;
 	}
@@ -178,7 +179,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
-			printf("Error line %d - no \\n found on string. "
+			RTE_LOG(ERR, CFGFILE, " line %d - no \\n found on string. "
 					"Check if line too long\n", lineno);
 			goto error1;
 		}
@@ -198,8 +199,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 			/* section heading line */
 			char *end = memchr(buffer, ']', len);
 			if (end == NULL) {
-				printf("Error line %d - no terminating ']'"
-					"character found\n", lineno);
+				RTE_LOG(ERR, CFGFILE,
+					"line %d - no terminating ']' character found\n",
+					lineno);
 				goto error1;
 			}
 			*end = '\0';
@@ -213,8 +215,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 			split[0] = buffer;
 			split[1] = memchr(buffer, '=', len);
 			if (split[1] == NULL) {
-				printf("Error line %d - no '='"
-					"character found\n", lineno);
+				RTE_LOG(ERR, CFGFILE,
+					"line %d - no '=' character found\n",
+					lineno);
 				goto error1;
 			}
 			*split[1] = '\0';
@@ -236,8 +239,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 
 			if (!(flags & CFG_FLAG_EMPTY_VALUES) &&
 					(*split[1] == '\0')) {
-				printf("Error at line %d - cannot use empty "
-							"values\n", lineno);
+				RTE_LOG(ERR, CFGFILE,
+					"line %d - cannot use empty values\n",
+					lineno);
 				goto error1;
 			}
 
@@ -397,7 +401,8 @@ int rte_cfgfile_set_entry(struct rte_cfgfile *cfg, const char *sectionname,
 				sizeof(curr_section->entries[i].value));
 			return 0;
 		}
-	printf("Error - entry name doesn't exist\n");
+
+	RTE_LOG(ERR, CFGFILE, "entry name doesn't exist\n");
 	return -EINVAL;
 }
 
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index cbb41846aaa3..fa747d5b90ef 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -62,6 +62,7 @@ extern struct rte_logs rte_logs;
 #define RTE_LOGTYPE_EFD       18 /**< Log related to EFD. */
 #define RTE_LOGTYPE_EVENTDEV  19 /**< Log related to eventdev. */
 #define RTE_LOGTYPE_GSO       20 /**< Log related to GSO. */
+#define RTE_LOGTYPE_CFGFILE   21 /**< Log related to cfgfile. */
 
 /* these log types can be used in an application */
 #define RTE_LOGTYPE_USER1     24 /**< User-defined log type 1. */
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH 3/3] cfgfile: use calloc
  2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
@ 2019-07-16 17:27 ` Stephen Hemminger
  2019-07-17 13:38 ` [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Bruce Richardson
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-16 17:27 UTC (permalink / raw)
  To: cristian.dumitrescu; +Cc: dev, Stephen Hemminger

Better to use calloc when allocating arrays.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index c4b768b6833f..39fec4b82bce 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -275,17 +275,16 @@ rte_cfgfile_create(int flags)
 	cfg->num_sections = 0;
 
 	/* allocate first batch of sections and entries */
-	cfg->sections = malloc(sizeof(struct rte_cfgfile_section) *
-			CFG_ALLOC_SECTION_BATCH);
-
+	cfg->sections = calloc(CFG_ALLOC_SECTION_BATCH,
+			       sizeof(struct rte_cfgfile_section));
 	if (cfg->sections == NULL)
 		goto error1;
 
 	cfg->allocated_sections = CFG_ALLOC_SECTION_BATCH;
 
 	for (i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
-		cfg->sections[i].entries = malloc(sizeof(
-			struct rte_cfgfile_entry) * CFG_ALLOC_ENTRY_BATCH);
+		cfg->sections[i].entries = calloc(CFG_ALLOC_ENTRY_BATCH,
+					  sizeof(struct rte_cfgfile_entry));
 
 		if (cfg->sections[i].entries == NULL)
 			goto error1;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches
  2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
                   ` (2 preceding siblings ...)
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 3/3] cfgfile: use calloc Stephen Hemminger
@ 2019-07-17 13:38 ` Bruce Richardson
  2019-07-18  0:48 ` [dpdk-dev] [PATCH v2 " Stephen Hemminger
  2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
  5 siblings, 0 replies; 22+ messages in thread
From: Bruce Richardson @ 2019-07-17 13:38 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: cristian.dumitrescu, dev

On Tue, Jul 16, 2019 at 10:27:38AM -0700, Stephen Hemminger wrote:
> Some small patches for the cfgfile library
> 
> Stephen Hemminger (3):
>   cfgfile: remove unnecessary initialization
>   cfgfile: use RTE_LOG for errors
>   cfgfile: use calloc
> 
Looks ok to me, and compile testing showed no issues on my system.

Series-Acked-by: Bruce Richardson <bruce.richardson@intel.com>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors
  2019-07-16 17:27 ` [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
@ 2019-07-17 21:01   ` Thomas Monjalon
  2019-07-17 22:10     ` Stephen Hemminger
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Monjalon @ 2019-07-17 21:01 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: dev, cristian.dumitrescu, david.marchand, bruce.richardson

16/07/2019 19:27, Stephen Hemminger:
> In general, DPDK libraries to not print error messages to
> stdout because that is often redirected to /dev/null for daemons.
> This patch changes cfgfile library to use RTE_LOG with its
> own type.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> --- a/lib/librte_eal/common/include/rte_log.h
> +++ b/lib/librte_eal/common/include/rte_log.h
> @@ -62,6 +62,7 @@ extern struct rte_logs rte_logs;
>  #define RTE_LOGTYPE_EFD       18 /**< Log related to EFD. */
>  #define RTE_LOGTYPE_EVENTDEV  19 /**< Log related to eventdev. */
>  #define RTE_LOGTYPE_GSO       20 /**< Log related to GSO. */
> +#define RTE_LOGTYPE_CFGFILE   21 /**< Log related to cfgfile. */

As you know, we are supposed to use dynamic logging now.
Let's stop to add new static log types here.
Better, we should plan to completely drop these types.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors
  2019-07-17 21:01   ` Thomas Monjalon
@ 2019-07-17 22:10     ` Stephen Hemminger
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-17 22:10 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, cristian.dumitrescu, david.marchand, bruce.richardson

On Wed, 17 Jul 2019 23:01:00 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 16/07/2019 19:27, Stephen Hemminger:
> > In general, DPDK libraries to not print error messages to
> > stdout because that is often redirected to /dev/null for daemons.
> > This patch changes cfgfile library to use RTE_LOG with its
> > own type.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > --- a/lib/librte_eal/common/include/rte_log.h
> > +++ b/lib/librte_eal/common/include/rte_log.h
> > @@ -62,6 +62,7 @@ extern struct rte_logs rte_logs;
> >  #define RTE_LOGTYPE_EFD       18 /**< Log related to EFD. */
> >  #define RTE_LOGTYPE_EVENTDEV  19 /**< Log related to eventdev. */
> >  #define RTE_LOGTYPE_GSO       20 /**< Log related to GSO. */
> > +#define RTE_LOGTYPE_CFGFILE   21 /**< Log related to cfgfile. */  
> 
> As you know, we are supposed to use dynamic logging now.
> Let's stop to add new static log types here.
> Better, we should plan to completely drop these types.
> 
> 

Ok, but rte_cfgfile can be used before eal init.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 0/3] cfgfile: cleanup patches
  2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
                   ` (3 preceding siblings ...)
  2019-07-17 13:38 ` [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Bruce Richardson
@ 2019-07-18  0:48 ` " Stephen Hemminger
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
                     ` (2 more replies)
  2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
  5 siblings, 3 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18  0:48 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Small cleanups to cfgfile library

Stephen Hemminger (3):
  cfgfile: remove unnecessary initialization
  cfgfile: use RTE_LOG for errors
  cfgfile: use calloc

 lib/librte_cfgfile/Makefile      |  1 +
 lib/librte_cfgfile/rte_cfgfile.c | 55 +++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 19 deletions(-)

v2 - use dynamic logtype instead of introducing new static value

-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 1/3] cfgfile: remove unnecessary initialization
  2019-07-18  0:48 ` [dpdk-dev] [PATCH v2 " Stephen Hemminger
@ 2019-07-18  0:48   ` Stephen Hemminger
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 3/3] cfgfile: use calloc Stephen Hemminger
  2 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18  0:48 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

No need to initialize variable if it is immediately overwritten.
It is better style not do unnecessary initialization with modern
tools since it lets compiler and other static checkers detect
uninitialized data.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index f8e7627a5169..1ef298592fa5 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -160,9 +160,9 @@ struct rte_cfgfile *
 rte_cfgfile_load_with_params(const char *filename, int flags,
 			     const struct rte_cfgfile_parameters *params)
 {
-	char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4] = {0};
+	char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4];
 	int lineno = 0;
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 
 	if (rte_cfgfile_check_params(params))
 		return NULL;
@@ -174,7 +174,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 	cfg = rte_cfgfile_create(flags);
 
 	while (fgets(buffer, sizeof(buffer), f) != NULL) {
-		char *pos = NULL;
+		char *pos;
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
@@ -260,7 +260,7 @@ struct rte_cfgfile *
 rte_cfgfile_create(int flags)
 {
 	int i;
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 
 	cfg = malloc(sizeof(*cfg));
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18  0:48 ` [dpdk-dev] [PATCH v2 " Stephen Hemminger
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
@ 2019-07-18  0:48   ` Stephen Hemminger
  2019-07-18  8:31     ` Bruce Richardson
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 3/3] cfgfile: use calloc Stephen Hemminger
  2 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18  0:48 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

In general, DPDK libraries to not print error messages to
stdout because that is often redirected to /dev/null for daemons.
This patch changes cfgfile library to use RTE_LOG with its
own type.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/Makefile      |  1 +
 lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
index d9512565e559..4fc711778699 100644
--- a/lib/librte_cfgfile/Makefile
+++ b/lib/librte_cfgfile/Makefile
@@ -11,6 +11,7 @@ LIB = librte_cfgfile.a
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
 CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
+LDFLAGS += -lrte_log
 
 EXPORT_MAP := rte_cfgfile_version.map
 
diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 1ef298592fa5..388415147930 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <rte_string_fns.h>
 #include <rte_common.h>
+#include <rte_log.h>
 
 #include "rte_cfgfile.h"
 
@@ -26,6 +27,12 @@ struct rte_cfgfile {
 	struct rte_cfgfile_section *sections;
 };
 
+static int cfgfile_logtype;
+
+#define CFG_LOG(level, fmt, args...)					\
+	rte_log(RTE_LOG_ ## level, cfgfile_logtype, "%s(): " fmt "\n",	\
+		__func__, ## args)
+
 /** when we resize a file structure, how many extra entries
  * for new sections do we add in */
 #define CFG_ALLOC_SECTION_BATCH 8
@@ -128,7 +135,7 @@ rte_cfgfile_check_params(const struct rte_cfgfile_parameters *params)
 	unsigned int i;
 
 	if (!params) {
-		printf("Error - missing cfgfile parameters\n");
+		CFG_LOG(ERR, "missing cfgfile parameters\n");
 		return -EINVAL;
 	}
 
@@ -141,7 +148,7 @@ rte_cfgfile_check_params(const struct rte_cfgfile_parameters *params)
 	}
 
 	if (valid_comment == 0)	{
-		printf("Error - invalid comment characters %c\n",
+		CFG_LOG(ERR, "invalid comment characters %c\n",
 		       params->comment_character);
 		return -ENOTSUP;
 	}
@@ -178,7 +185,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
-			printf("Error line %d - no \\n found on string. "
+			CFG_LOG(ERR, " line %d - no \\n found on string. "
 					"Check if line too long\n", lineno);
 			goto error1;
 		}
@@ -198,8 +205,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 			/* section heading line */
 			char *end = memchr(buffer, ']', len);
 			if (end == NULL) {
-				printf("Error line %d - no terminating ']'"
-					"character found\n", lineno);
+				CFG_LOG(ERR,
+					"line %d - no terminating ']' character found\n",
+					lineno);
 				goto error1;
 			}
 			*end = '\0';
@@ -213,8 +221,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 			split[0] = buffer;
 			split[1] = memchr(buffer, '=', len);
 			if (split[1] == NULL) {
-				printf("Error line %d - no '='"
-					"character found\n", lineno);
+				CFG_LOG(ERR,
+					"line %d - no '=' character found\n",
+					lineno);
 				goto error1;
 			}
 			*split[1] = '\0';
@@ -236,8 +245,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 
 			if (!(flags & CFG_FLAG_EMPTY_VALUES) &&
 					(*split[1] == '\0')) {
-				printf("Error at line %d - cannot use empty "
-							"values\n", lineno);
+				CFG_LOG(ERR,
+					"line %d - cannot use empty values\n",
+					lineno);
 				goto error1;
 			}
 
@@ -397,7 +407,8 @@ int rte_cfgfile_set_entry(struct rte_cfgfile *cfg, const char *sectionname,
 				sizeof(curr_section->entries[i].value));
 			return 0;
 		}
-	printf("Error - entry name doesn't exist\n");
+
+	CFG_LOG(ERR, "entry name doesn't exist\n");
 	return -EINVAL;
 }
 
@@ -552,3 +563,10 @@ rte_cfgfile_has_entry(struct rte_cfgfile *cfg, const char *sectionname,
 {
 	return rte_cfgfile_get_entry(cfg, sectionname, entryname) != NULL;
 }
+
+RTE_INIT(cfgfile_init)
+{
+	cfgfile_logtype = rte_log_register("lib.cfgfile");
+	if (cfgfile_logtype >= 0)
+		rte_log_set_level(cfgfile_logtype, RTE_LOG_INFO);
+}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v2 3/3] cfgfile: use calloc
  2019-07-18  0:48 ` [dpdk-dev] [PATCH v2 " Stephen Hemminger
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
@ 2019-07-18  0:48   ` Stephen Hemminger
  2 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18  0:48 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Better to use calloc when allocating arrays.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 388415147930..9049fd9c2319 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -281,17 +281,16 @@ rte_cfgfile_create(int flags)
 	cfg->num_sections = 0;
 
 	/* allocate first batch of sections and entries */
-	cfg->sections = malloc(sizeof(struct rte_cfgfile_section) *
-			CFG_ALLOC_SECTION_BATCH);
-
+	cfg->sections = calloc(CFG_ALLOC_SECTION_BATCH,
+			       sizeof(struct rte_cfgfile_section));
 	if (cfg->sections == NULL)
 		goto error1;
 
 	cfg->allocated_sections = CFG_ALLOC_SECTION_BATCH;
 
 	for (i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
-		cfg->sections[i].entries = malloc(sizeof(
-			struct rte_cfgfile_entry) * CFG_ALLOC_ENTRY_BATCH);
+		cfg->sections[i].entries = calloc(CFG_ALLOC_ENTRY_BATCH,
+					  sizeof(struct rte_cfgfile_entry));
 
 		if (cfg->sections[i].entries == NULL)
 			goto error1;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
@ 2019-07-18  8:31     ` Bruce Richardson
  2019-07-18 14:34       ` Stephen Hemminger
  0 siblings, 1 reply; 22+ messages in thread
From: Bruce Richardson @ 2019-07-18  8:31 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Wed, Jul 17, 2019 at 05:48:21PM -0700, Stephen Hemminger wrote:
> In general, DPDK libraries to not print error messages to
> stdout because that is often redirected to /dev/null for daemons.
> This patch changes cfgfile library to use RTE_LOG with its
> own type.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_cfgfile/Makefile      |  1 +
>  lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
> index d9512565e559..4fc711778699 100644
> --- a/lib/librte_cfgfile/Makefile
> +++ b/lib/librte_cfgfile/Makefile
> @@ -11,6 +11,7 @@ LIB = librte_cfgfile.a
>  CFLAGS += -O3
>  CFLAGS += $(WERROR_FLAGS)
>  CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
> +LDFLAGS += -lrte_log
>  
Where does this come from, there is no separate log library in DPDK?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18  8:31     ` Bruce Richardson
@ 2019-07-18 14:34       ` Stephen Hemminger
  2019-07-18 14:36         ` Bruce Richardson
  0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 14:34 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Thu, 18 Jul 2019 09:31:11 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, Jul 17, 2019 at 05:48:21PM -0700, Stephen Hemminger wrote:
> > In general, DPDK libraries to not print error messages to
> > stdout because that is often redirected to /dev/null for daemons.
> > This patch changes cfgfile library to use RTE_LOG with its
> > own type.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/librte_cfgfile/Makefile      |  1 +
> >  lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
> >  2 files changed, 29 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
> > index d9512565e559..4fc711778699 100644
> > --- a/lib/librte_cfgfile/Makefile
> > +++ b/lib/librte_cfgfile/Makefile
> > @@ -11,6 +11,7 @@ LIB = librte_cfgfile.a
> >  CFLAGS += -O3
> >  CFLAGS += $(WERROR_FLAGS)
> >  CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
> > +LDFLAGS += -lrte_log
> >    
> Where does this come from, there is no separate log library in DPDK?

I saw a build failure with previous patch about rte_log not being
present.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18 14:34       ` Stephen Hemminger
@ 2019-07-18 14:36         ` Bruce Richardson
  2019-07-18 14:41           ` Stephen Hemminger
  2019-07-18 17:12           ` Stephen Hemminger
  0 siblings, 2 replies; 22+ messages in thread
From: Bruce Richardson @ 2019-07-18 14:36 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Thu, Jul 18, 2019 at 07:34:59AM -0700, Stephen Hemminger wrote:
> On Thu, 18 Jul 2019 09:31:11 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> > On Wed, Jul 17, 2019 at 05:48:21PM -0700, Stephen Hemminger wrote:
> > > In general, DPDK libraries to not print error messages to
> > > stdout because that is often redirected to /dev/null for daemons.
> > > This patch changes cfgfile library to use RTE_LOG with its
> > > own type.
> > > 
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  lib/librte_cfgfile/Makefile      |  1 +
> > >  lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
> > >  2 files changed, 29 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
> > > index d9512565e559..4fc711778699 100644
> > > --- a/lib/librte_cfgfile/Makefile
> > > +++ b/lib/librte_cfgfile/Makefile
> > > @@ -11,6 +11,7 @@ LIB = librte_cfgfile.a
> > >  CFLAGS += -O3
> > >  CFLAGS += $(WERROR_FLAGS)
> > >  CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
> > > +LDFLAGS += -lrte_log
> > >    
> > Where does this come from, there is no separate log library in DPDK?
> 
> I saw a build failure with previous patch about rte_log not being
> present.

I can believe that, it's just how does this help? What does the linker pick
up when you pass this?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18 14:36         ` Bruce Richardson
@ 2019-07-18 14:41           ` Stephen Hemminger
  2019-07-18 17:12           ` Stephen Hemminger
  1 sibling, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 14:41 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Thu, 18 Jul 2019 15:36:45 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Thu, Jul 18, 2019 at 07:34:59AM -0700, Stephen Hemminger wrote:
> > On Thu, 18 Jul 2019 09:31:11 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >   
> > > On Wed, Jul 17, 2019 at 05:48:21PM -0700, Stephen Hemminger wrote:  
> > > > In general, DPDK libraries to not print error messages to
> > > > stdout because that is often redirected to /dev/null for daemons.
> > > > This patch changes cfgfile library to use RTE_LOG with its
> > > > own type.
> > > > 
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > ---
> > > >  lib/librte_cfgfile/Makefile      |  1 +
> > > >  lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
> > > >  2 files changed, 29 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
> > > > index d9512565e559..4fc711778699 100644
> > > > --- a/lib/librte_cfgfile/Makefile
> > > > +++ b/lib/librte_cfgfile/Makefile
> > > > @@ -11,6 +11,7 @@ LIB = librte_cfgfile.a
> > > >  CFLAGS += -O3
> > > >  CFLAGS += $(WERROR_FLAGS)
> > > >  CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
> > > > +LDFLAGS += -lrte_log
> > > >      
> > > Where does this come from, there is no separate log library in DPDK?  
> > 
> > I saw a build failure with previous patch about rte_log not being
> > present.  
> 
> I can believe that, it's just how does this help? What does the linker pick
> up when you pass this?

I don't think it helps, it was an ICC only error and it was purely
a guess since I don't have ICC.

Let me take it off in the next version and see what CI says.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18 14:36         ` Bruce Richardson
  2019-07-18 14:41           ` Stephen Hemminger
@ 2019-07-18 17:12           ` Stephen Hemminger
  1 sibling, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 17:12 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev

On Thu, 18 Jul 2019 15:36:45 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Thu, Jul 18, 2019 at 07:34:59AM -0700, Stephen Hemminger wrote:
> > On Thu, 18 Jul 2019 09:31:11 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >   
> > > On Wed, Jul 17, 2019 at 05:48:21PM -0700, Stephen Hemminger wrote:  
> > > > In general, DPDK libraries to not print error messages to
> > > > stdout because that is often redirected to /dev/null for daemons.
> > > > This patch changes cfgfile library to use RTE_LOG with its
> > > > own type.
> > > > 
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > > ---
> > > >  lib/librte_cfgfile/Makefile      |  1 +
> > > >  lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
> > > >  2 files changed, 29 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/lib/librte_cfgfile/Makefile b/lib/librte_cfgfile/Makefile
> > > > index d9512565e559..4fc711778699 100644
> > > > --- a/lib/librte_cfgfile/Makefile
> > > > +++ b/lib/librte_cfgfile/Makefile
> > > > @@ -11,6 +11,7 @@ LIB = librte_cfgfile.a
> > > >  CFLAGS += -O3
> > > >  CFLAGS += $(WERROR_FLAGS)
> > > >  CFLAGS += -I$(SRCDIR)/../librte_eal/common/include
> > > > +LDFLAGS += -lrte_log
> > > >      
> > > Where does this come from, there is no separate log library in DPDK?  
> > 
> > I saw a build failure with previous patch about rte_log not being
> > present.  
> 
> I can believe that, it's just how does this help? What does the linker pick
> up when you pass this?


Not sure what causes this:

*Make Build Failed #1:
OS: RHEL80-64
Target: x86_64-native-linuxapp-gcc+shared
rte_cfgfile.o:rte_cfgfile.c:(.text.unlikely+0xbe): more undefined references to `rte_log' follow
rte_cfgfile.o: In function `cfgfile_init':
rte_cfgfile.c:(.text.startup+0xc): undefined reference to `rte_log_register'
rte_cfgfile.c:(.text.startup+0x25): undefined reference to `rte_log_set_level'
collect2: error: ld returned 1 exit status
make[5]: *** [/tmp/RHEL80-64_K3.10.0_GCC8.2.1/x86_64-native-linuxapp-gcc+shared/6baa0548e7644f418e02f19043f86f82/dpdk/mk/rte.lib.mk:102: librte_cfgfile.so.2.1] Error 1
make[4]: *** [/tmp/RHEL80-64_K3.10.0_GCC8.2.1/x86_64-native-linuxapp-gcc+shared/6baa0548e7644f418e02f19043f86f82/dpdk/mk/rte.subdir.mk:37: librte_cfgfile] Error 2
make[4]: *** Waiting for unfinished jobs....
== Build lib/librte_eal/common
  SYMLINK-FILE include/rte_common.h
  SYMLINK-FILE include/rte_compat.h
  SYMLINK-FILE include/rte_branch_prediction.h
--

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches
  2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
                   ` (4 preceding siblings ...)
  2019-07-18  0:48 ` [dpdk-dev] [PATCH v2 " Stephen Hemminger
@ 2019-07-18 17:18 ` Stephen Hemminger
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
                     ` (3 more replies)
  5 siblings, 4 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 17:18 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Small cleanups to cfgfile library.

Stephen Hemminger (3):
  cfgfile: remove unnecessary initialization
  cfgfile: use RTE_LOG for errors
  cfgfile: use calloc

 lib/Makefile                     |  1 +
 lib/librte_cfgfile/rte_cfgfile.c | 55 +++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 19 deletions(-)

v3 - cfgfile now depends on rte_log from eal so fix Makefile
v2 - use dynamic logtype instead of introducing new static value

-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v3 1/3] cfgfile: remove unnecessary initialization
  2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
@ 2019-07-18 17:18   ` Stephen Hemminger
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 17:18 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

No need to initialize variable if it is immediately overwritten.
It is better style not do unnecessary initialization with modern
tools since it lets compiler and other static checkers detect
uninitialized data.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index f8e7627a5169..1ef298592fa5 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -160,9 +160,9 @@ struct rte_cfgfile *
 rte_cfgfile_load_with_params(const char *filename, int flags,
 			     const struct rte_cfgfile_parameters *params)
 {
-	char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4] = {0};
+	char buffer[CFG_NAME_LEN + CFG_VALUE_LEN + 4];
 	int lineno = 0;
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 
 	if (rte_cfgfile_check_params(params))
 		return NULL;
@@ -174,7 +174,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 	cfg = rte_cfgfile_create(flags);
 
 	while (fgets(buffer, sizeof(buffer), f) != NULL) {
-		char *pos = NULL;
+		char *pos;
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
@@ -260,7 +260,7 @@ struct rte_cfgfile *
 rte_cfgfile_create(int flags)
 {
 	int i;
-	struct rte_cfgfile *cfg = NULL;
+	struct rte_cfgfile *cfg;
 
 	cfg = malloc(sizeof(*cfg));
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v3 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
@ 2019-07-18 17:18   ` Stephen Hemminger
  2019-07-18 22:39     ` Thomas Monjalon
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 3/3] cfgfile: use calloc Stephen Hemminger
  2019-07-18 22:44   ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Thomas Monjalon
  3 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 17:18 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

In general, DPDK libraries to not print error messages to
stdout because that is often redirected to /dev/null for daemons.
This patch changes cfgfile library to use RTE_LOG with its
own type.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/Makefile                     |  1 +
 lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/lib/Makefile b/lib/Makefile
index bdb8a67e2d9d..41c463d92139 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -19,6 +19,7 @@ DEPDIRS-librte_mbuf := librte_eal librte_mempool
 DIRS-$(CONFIG_RTE_LIBRTE_TIMER) += librte_timer
 DEPDIRS-librte_timer := librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += librte_cfgfile
+DEPDIRS-librte_cfgfile := librte_eal
 DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += librte_cmdline
 DEPDIRS-librte_cmdline := librte_eal librte_net
 DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ethdev
diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 1ef298592fa5..388415147930 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -9,6 +9,7 @@
 #include <errno.h>
 #include <rte_string_fns.h>
 #include <rte_common.h>
+#include <rte_log.h>
 
 #include "rte_cfgfile.h"
 
@@ -26,6 +27,12 @@ struct rte_cfgfile {
 	struct rte_cfgfile_section *sections;
 };
 
+static int cfgfile_logtype;
+
+#define CFG_LOG(level, fmt, args...)					\
+	rte_log(RTE_LOG_ ## level, cfgfile_logtype, "%s(): " fmt "\n",	\
+		__func__, ## args)
+
 /** when we resize a file structure, how many extra entries
  * for new sections do we add in */
 #define CFG_ALLOC_SECTION_BATCH 8
@@ -128,7 +135,7 @@ rte_cfgfile_check_params(const struct rte_cfgfile_parameters *params)
 	unsigned int i;
 
 	if (!params) {
-		printf("Error - missing cfgfile parameters\n");
+		CFG_LOG(ERR, "missing cfgfile parameters\n");
 		return -EINVAL;
 	}
 
@@ -141,7 +148,7 @@ rte_cfgfile_check_params(const struct rte_cfgfile_parameters *params)
 	}
 
 	if (valid_comment == 0)	{
-		printf("Error - invalid comment characters %c\n",
+		CFG_LOG(ERR, "invalid comment characters %c\n",
 		       params->comment_character);
 		return -ENOTSUP;
 	}
@@ -178,7 +185,7 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 		size_t len = strnlen(buffer, sizeof(buffer));
 		lineno++;
 		if ((len >= sizeof(buffer) - 1) && (buffer[len-1] != '\n')) {
-			printf("Error line %d - no \\n found on string. "
+			CFG_LOG(ERR, " line %d - no \\n found on string. "
 					"Check if line too long\n", lineno);
 			goto error1;
 		}
@@ -198,8 +205,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 			/* section heading line */
 			char *end = memchr(buffer, ']', len);
 			if (end == NULL) {
-				printf("Error line %d - no terminating ']'"
-					"character found\n", lineno);
+				CFG_LOG(ERR,
+					"line %d - no terminating ']' character found\n",
+					lineno);
 				goto error1;
 			}
 			*end = '\0';
@@ -213,8 +221,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 			split[0] = buffer;
 			split[1] = memchr(buffer, '=', len);
 			if (split[1] == NULL) {
-				printf("Error line %d - no '='"
-					"character found\n", lineno);
+				CFG_LOG(ERR,
+					"line %d - no '=' character found\n",
+					lineno);
 				goto error1;
 			}
 			*split[1] = '\0';
@@ -236,8 +245,9 @@ rte_cfgfile_load_with_params(const char *filename, int flags,
 
 			if (!(flags & CFG_FLAG_EMPTY_VALUES) &&
 					(*split[1] == '\0')) {
-				printf("Error at line %d - cannot use empty "
-							"values\n", lineno);
+				CFG_LOG(ERR,
+					"line %d - cannot use empty values\n",
+					lineno);
 				goto error1;
 			}
 
@@ -397,7 +407,8 @@ int rte_cfgfile_set_entry(struct rte_cfgfile *cfg, const char *sectionname,
 				sizeof(curr_section->entries[i].value));
 			return 0;
 		}
-	printf("Error - entry name doesn't exist\n");
+
+	CFG_LOG(ERR, "entry name doesn't exist\n");
 	return -EINVAL;
 }
 
@@ -552,3 +563,10 @@ rte_cfgfile_has_entry(struct rte_cfgfile *cfg, const char *sectionname,
 {
 	return rte_cfgfile_get_entry(cfg, sectionname, entryname) != NULL;
 }
+
+RTE_INIT(cfgfile_init)
+{
+	cfgfile_logtype = rte_log_register("lib.cfgfile");
+	if (cfgfile_logtype >= 0)
+		rte_log_set_level(cfgfile_logtype, RTE_LOG_INFO);
+}
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [dpdk-dev] [PATCH v3 3/3] cfgfile: use calloc
  2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
@ 2019-07-18 17:18   ` Stephen Hemminger
  2019-07-18 22:44   ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Thomas Monjalon
  3 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2019-07-18 17:18 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Better to use calloc when allocating arrays.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 388415147930..9049fd9c2319 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -281,17 +281,16 @@ rte_cfgfile_create(int flags)
 	cfg->num_sections = 0;
 
 	/* allocate first batch of sections and entries */
-	cfg->sections = malloc(sizeof(struct rte_cfgfile_section) *
-			CFG_ALLOC_SECTION_BATCH);
-
+	cfg->sections = calloc(CFG_ALLOC_SECTION_BATCH,
+			       sizeof(struct rte_cfgfile_section));
 	if (cfg->sections == NULL)
 		goto error1;
 
 	cfg->allocated_sections = CFG_ALLOC_SECTION_BATCH;
 
 	for (i = 0; i < CFG_ALLOC_SECTION_BATCH; i++) {
-		cfg->sections[i].entries = malloc(sizeof(
-			struct rte_cfgfile_entry) * CFG_ALLOC_ENTRY_BATCH);
+		cfg->sections[i].entries = calloc(CFG_ALLOC_ENTRY_BATCH,
+					  sizeof(struct rte_cfgfile_entry));
 
 		if (cfg->sections[i].entries == NULL)
 			goto error1;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/3] cfgfile: use RTE_LOG for errors
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
@ 2019-07-18 22:39     ` Thomas Monjalon
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2019-07-18 22:39 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

18/07/2019 19:18, Stephen Hemminger:
> In general, DPDK libraries to not print error messages to
> stdout because that is often redirected to /dev/null for daemons.
> This patch changes cfgfile library to use RTE_LOG with its
> own type.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/Makefile                     |  1 +
>  lib/librte_cfgfile/rte_cfgfile.c | 38 +++++++++++++++++++++++---------
>  2 files changed, 29 insertions(+), 10 deletions(-)
> 
> --- a/lib/Makefile
> +++ b/lib/Makefile
>  DIRS-$(CONFIG_RTE_LIBRTE_CFGFILE) += librte_cfgfile
> +DEPDIRS-librte_cfgfile := librte_eal

You are missing LDLIBS += -lrte_eal in the librte_cfgfile Makefile.

I can add it while applying.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches
  2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
                     ` (2 preceding siblings ...)
  2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 3/3] cfgfile: use calloc Stephen Hemminger
@ 2019-07-18 22:44   ` Thomas Monjalon
  3 siblings, 0 replies; 22+ messages in thread
From: Thomas Monjalon @ 2019-07-18 22:44 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

18/07/2019 19:18, Stephen Hemminger:
> Small cleanups to cfgfile library.
> 
> Stephen Hemminger (3):
>   cfgfile: remove unnecessary initialization
>   cfgfile: use RTE_LOG for errors
>   cfgfile: use calloc
> 
>  lib/Makefile                     |  1 +
>  lib/librte_cfgfile/rte_cfgfile.c | 55 +++++++++++++++++++++-----------
>  2 files changed, 37 insertions(+), 19 deletions(-)
> 
> v3 - cfgfile now depends on rte_log from eal so fix Makefile
> v2 - use dynamic logtype instead of introducing new static value

Applied with a fix in Makefile, thanks



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, back to index

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16 17:27 [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Stephen Hemminger
2019-07-16 17:27 ` [dpdk-dev] [PATCH 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
2019-07-16 17:27 ` [dpdk-dev] [PATCH 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
2019-07-17 21:01   ` Thomas Monjalon
2019-07-17 22:10     ` Stephen Hemminger
2019-07-16 17:27 ` [dpdk-dev] [PATCH 3/3] cfgfile: use calloc Stephen Hemminger
2019-07-17 13:38 ` [dpdk-dev] [PATCH 0/3] cfgfile: cleanup patches Bruce Richardson
2019-07-18  0:48 ` [dpdk-dev] [PATCH v2 " Stephen Hemminger
2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
2019-07-18  8:31     ` Bruce Richardson
2019-07-18 14:34       ` Stephen Hemminger
2019-07-18 14:36         ` Bruce Richardson
2019-07-18 14:41           ` Stephen Hemminger
2019-07-18 17:12           ` Stephen Hemminger
2019-07-18  0:48   ` [dpdk-dev] [PATCH v2 3/3] cfgfile: use calloc Stephen Hemminger
2019-07-18 17:18 ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Stephen Hemminger
2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 1/3] cfgfile: remove unnecessary initialization Stephen Hemminger
2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 2/3] cfgfile: use RTE_LOG for errors Stephen Hemminger
2019-07-18 22:39     ` Thomas Monjalon
2019-07-18 17:18   ` [dpdk-dev] [PATCH v3 3/3] cfgfile: use calloc Stephen Hemminger
2019-07-18 22:44   ` [dpdk-dev] [PATCH v3 0/3] cfgfile: cleanup patches Thomas Monjalon

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox