[2/3] cfgfile: use RTE_LOG for errors

Message ID 20190716172741.21399-3-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series cfgfile: cleanup patches |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Stephen Hemminger July 16, 2019, 5:27 p.m. UTC
  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(-)
  

Comments

Thomas Monjalon July 17, 2019, 9:01 p.m. UTC | #1
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.
  
Stephen Hemminger July 17, 2019, 10:10 p.m. UTC | #2
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.
  

Patch

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. */