Allow empty prefix

PATCH New Zdenek Kabelac 2 participants 6 messages Oct 6, 2011 – Oct 26, 2011

Share & tools

Zdenek Kabelac

author

2011-10-06 09:42 UTC #0 lore raw

cover Allow empty prefix

Resolving problem of command like 'lvs/pvs/vgs' were always printing 2 space prefix " " even if user has specified no prefix.

1. fix coredump of lvm if buggy config is read.
(introduced with cascaded patch).

2. For reading empty string introduce find_config_tree_str_allow_empty() and dm_config_tree_find_str_allow_empty().

3. Change default to "" (no prefix) - since that what most users would expect here probably.

Zdenek Kabelac (3):
Fix core on buggy config file
Add find_config_tree_str_allow_empty
Change default prefix.

 doc/example.conf.in        |    6 +++---
 lib/commands/toolcontext.c |    7 ++++---
 lib/config/config.c        |    6 ++++++
 lib/config/config.h        |    2 ++
 lib/config/defaults.h      |    6 +++++-
 libdm/libdevmapper.h       |    2 ++
 libdm/libdm-config.c       |   25 +++++++++++++++++++------
 test/t-000-basic.sh        |    3 +++
 8 files changed, 44 insertions(+), 13 deletions(-)

Signature · 1 line

1.7.6.4

Zdenek Kabelac

author

2011-10-06 09:43 UTC #1 lore raw

PATCH 1/3 Fix core on buggy config file

Since fixed within unreleased version - no WHATS_NEW

Signed-off-by Zdenek Kabelac <[email protected]>

---
 libdm/libdm-config.c |    6 +++++-
 test/t-000-basic.sh  |    3 +++
 2 files changed, 8 insertions(+), 1 deletions(-)

libdm/libdm-config.c +5 -1

index a831936..b9ce7a3 100644
@@ -170,8 +170,12 @@ void dm_config_destroy(struct dm_config_tree *cft)
  */
 struct dm_config_tree *dm_config_remove_cascaded_tree(struct dm_config_tree *cft)
 {
-	struct dm_config_tree *second_cft = cft->cascade;
+	struct dm_config_tree *second_cft;
 
+	if (!cft)
+		return NULL;
+
+	second_cft = cft->cascade;
 	cft->cascade = NULL;
 
 	return second_cft;

test/t-000-basic.sh +3 -0

index bd7ba69..dbee47c 100755
@@ -23,3 +23,6 @@ diff -u actual expected
 # ensure we can create devices (uses dmsetup, etc)
 aux prepare_devs 5
 
+# ensure we not crash on buggy in config files
+aux lvmconf 'log/prefix = 1""'
+not lvs

Signature · 1 line

1.7.6.4

Zdenek Kabelac

author

2011-10-06 09:43 UTC #2 lore raw

PATCH 2/3 Add find_config_tree_str_allow_empty

In reply to Zdenek Kabelac

Add function to allow read of empty strings as valid arguments. Add a warning message if string argument has ignored value.

Signed-off-by Zdenek Kabelac <[email protected]>

---
 lib/commands/toolcontext.c |    7 ++++---
 lib/config/config.c        |    6 ++++++
 lib/config/config.h        |    2 ++
 libdm/libdevmapper.h       |    2 ++
 libdm/libdm-config.c       |   19 ++++++++++++++-----
 5 files changed, 28 insertions(+), 8 deletions(-)

lib/commands/toolcontext.c +4 -3

index 6c554b2..5ab2c83 100644
@@ -127,6 +127,7 @@ static void _init_logging(struct cmd_context *cmd)
 
 	const char *log_file;
 	char timebuf[26];
+	const struct dm_config_node *cn;
 
 	/* Syslog */
 	cmd->default_settings.syslog =

@@ -153,9 +154,9 @@ static void _init_logging(struct cmd_context *cmd)
 	init_abort_on_internal_errors(find_config_tree_int(cmd, "global/abort_on_internal_errors",
 							   DEFAULT_ABORT_ON_INTERNAL_ERRORS));
 
-	cmd->default_settings.msg_prefix = find_config_tree_str(cmd,
-							   "log/prefix",
-							   DEFAULT_MSG_PREFIX);
+	cmd->default_settings.msg_prefix =
+		find_config_tree_str_allow_empty(cmd, "log/prefix", DEFAULT_MSG_PREFIX);
+
 	init_msg_prefix(cmd->default_settings.msg_prefix);
 
 	cmd->default_settings.cmd_name = find_config_tree_int(cmd,

lib/config/config.c +6 -0

index c62e959..d82fee4 100644
@@ -182,6 +182,12 @@ const char *find_config_tree_str(struct cmd_context *cmd,
 	return dm_config_tree_find_str(cmd->cft, path, fail);
 }
 
+const char *find_config_tree_str_allow_empty(struct cmd_context *cmd,
+					     const char *path, const char *fail)
+{
+	return dm_config_tree_find_str_allow_empty(cmd->cft, path, fail);
+}
+
 int find_config_tree_int(struct cmd_context *cmd, const char *path,
 			 int fail)
 {

lib/config/config.h +2 -0

index 5edc6cc..dd6daeb 100644
@@ -44,6 +44,8 @@ const struct dm_config_node *find_config_tree_node(struct cmd_context *cmd,
 						   const char *path);
 const char *find_config_tree_str(struct cmd_context *cmd,
 				 const char *path, const char *fail);
+const char *find_config_tree_str_allow_empty(struct cmd_context *cmd,
+					     const char *path, const char *fail);
 int find_config_tree_int(struct cmd_context *cmd, const char *path,
 			 int fail);
 int64_t find_config_tree_int64(struct cmd_context *cmd, const char *path,

libdm/libdevmapper.h +2 -0

index 223a0fe..84b586b 100644
@@ -1350,6 +1350,8 @@ float dm_config_find_float(const struct dm_config_node *cn, const char *path, fl
 const struct dm_config_node *dm_config_tree_find_node(const struct dm_config_tree *cft, const char *path);
 const char *dm_config_tree_find_str(const struct dm_config_tree *cft,
 				    const char *path, const char *fail);
+const char *dm_config_tree_find_str_allow_empty(const struct dm_config_tree *cft,
+						const char *path, const char *fail);
 int dm_config_tree_find_int(const struct dm_config_tree *cft,
 			    const char *path, int fail);
 int64_t dm_config_tree_find_int64(const struct dm_config_tree *cft,

libdm/libdm-config.c +14 -5

index b9ce7a3..3ece3d3 100644
@@ -923,15 +923,18 @@ static const struct dm_config_node *_find_first_config_node(const void *start, c
 }
 
 static const char *_find_config_str(const void *start, node_lookup_fn find_fn,
-				    const char *path, const char *fail)
+				    const char *path, const char *fail, int allow_empty)
 {
 	const struct dm_config_node *n = find_fn(start, path);
 
 	/* Empty strings are ignored */
-	if ((n && n->v && n->v->type == DM_CFG_STRING) && (*n->v->v.str)) {
+	if ((n && n->v && n->v->type == DM_CFG_STRING) &&
+	    (allow_empty || (*n->v->v.str))) {
 		log_very_verbose("Setting %s to %s", path, n->v->v.str);
 		return n->v->v.str;
-	}
+	} else if (n && (!n->v || (n->v->type != DM_CFG_STRING) ||
+			 (!allow_empty && fail)))
+		log_warn("WARNING: Ignoring unsupported value for %s.", path);
 
 	if (fail)
 		log_very_verbose("%s not found in config: defaulting to %s",

@@ -942,7 +945,7 @@ static const char *_find_config_str(const void *start, node_lookup_fn find_fn,
 const char *dm_config_find_str(const struct dm_config_node *cn,
 			       const char *path, const char *fail)
 {
-	return _find_config_str(cn, _find_config_node, path, fail);
+	return _find_config_str(cn, _find_config_node, path, fail, 0);
 }
 
 static int64_t _find_config_int64(const void *start, node_lookup_fn find,

@@ -1064,7 +1067,13 @@ const struct dm_config_node *dm_config_tree_find_node(const struct dm_config_tre
 const char *dm_config_tree_find_str(const struct dm_config_tree *cft, const char *path,
 				    const char *fail)
 {
-	return _find_config_str(cft, _find_first_config_node, path, fail);
+	return _find_config_str(cft, _find_first_config_node, path, fail, 0);
+}
+
+const char *dm_config_tree_find_str_allow_empty(const struct dm_config_tree *cft, const char *path,
+						const char *fail)
+{
+	return _find_config_str(cft, _find_first_config_node, path, fail, 1);
 }
 
 int dm_config_tree_find_int(const struct dm_config_tree *cft, const char *path, int fail)

Signature · 1 line

1.7.6.4

Zdenek Kabelac

author

2011-10-06 09:43 UTC #3 lore raw

PATCH 3/3 Change default prefix.

In reply to Zdenek Kabelac

Use "" instead of " ".
So commands like lvs print from the begining of the line. (no need to trim space around).

Signed-off-by Zdenek Kabelac <[email protected]>

---
 doc/example.conf.in   |    6 +++---
 lib/config/defaults.h |    6 +++++-
 2 files changed, 8 insertions(+), 4 deletions(-)

doc/example.conf.in +3 -3

index d38f90e..58748cc 100644
@@ -248,9 +248,9 @@ log {
     command_names = 0
 
     # A prefix to use before the message text (but after the command name,
-    # if selected).  Default is two spaces, so you can see/grep the severity
-    # of each message.
-    prefix = "  "
+    # if selected). Set to "  " to revert to the default behaviour
+    # prior to version 2.02.89.
+    prefix = ""
 
     # To make the messages look similar to the original LVM tools use:
     #   indent = 0

lib/config/defaults.h +5 -1

index be016f6..df1dcba 100644
@@ -99,7 +99,11 @@
 #define DEFAULT_MAXIMISE_CLING 1
 #define DEFAULT_CLUSTERED 0
 
-#define DEFAULT_MSG_PREFIX "  "
+/*
+ * NOTE: _find_config_str() does not allow empty string "",
+ * but when the default msg prefix is also "" it works as expected.
+ */
+#define DEFAULT_MSG_PREFIX ""
 #define DEFAULT_CMD_NAME 0
 #define DEFAULT_OVERWRITE 0

Signature · 1 line

1.7.6.4

Zdenek Kabelac

author

2011-10-06 09:49 UTC #4 lore raw

PATCH 3/3 Change default prefix.

Dne 6.10.2011 11:43, Zdenek Kabelac napsal(a):

Quoted 32 lines

Use "" instead of " ".
So commands like lvs print from the begining of the line. (no need to trim space around).

Signed-off-by Zdenek Kabelac<[email protected]>

---
  doc/example.conf.in   |    6 +++---
  lib/config/defaults.h |    6 +++++-
  2 files changed, 8 insertions(+), 4 deletions(-)

doc/example.conf.in +3 -3

index d38f90e..58748cc 100644
@@ -248,9 +248,9 @@ log {
      command_names = 0
 
      # A prefix to use before the message text (but after the command name,
-    # if selected).  Default is two spaces, so you can see/grep the severity
-    # of each message.
-    prefix = "  "
+    # if selected). Set to "  " to revert to the default behaviour
+    # prior to version 2.02.89.
+    prefix = ""
 
      # To make the messages look similar to the original LVM tools use:
      #   indent = 0

lib/config/defaults.h +0 -0

index be016f6..df1dcba 100644

lib/config/defaults.h line 106

Quoted 9 lines

lib/config/defaults.h +5 -1

@@ -99,7 +99,11 @@
  #define DEFAULT_MAXIMISE_CLING 1
  #define DEFAULT_CLUSTERED 0
 
-#define DEFAULT_MSG_PREFIX "  "
+/*
+ * NOTE: _find_config_str() does not allow empty string "",
+ * but when the default msg prefix is also "" it works as expected.
+ */
+#define DEFAULT_MSG_PREFIX ""

The comment here is not true anymore with patch 2/3 as I've reorderd patch set somewhat. So committed version will not have it.

Zdenek

Milan Broz

2011-10-26 11:19 UTC #5 lore raw

PATCH 3/3 Change default prefix.

In reply to Zdenek Kabelac

Quoted 15 lines · Zdenek Kabelac

Use "" instead of " ".
So commands like lvs print from the begining of the line. (no need to trim space around).
Signed-off-by Zdenek Kabelac <[email protected]>
---
 doc/example.conf.in   |    6 +++---
 lib/config/defaults.h |    6 +++++-
 2 files changed, 8 insertions(+), 4 deletions(-)
doc/example.conf.in +0 -0
index d38f90e..58748cc 100644

doc/example.conf.in log { · line 253

Quoted 9 lines

doc/example.conf.in +3 -3

@@ -248,9 +248,9 @@ log {
     command_names = 0
 
     # A prefix to use before the message text (but after the command name,
-    # if selected).  Default is two spaces, so you can see/grep the severity
-    # of each message.
-    prefix = "  "
+    # if selected). Set to "  " to revert to the default behaviour
+    # prior to version 2.02.89.
+    prefix = ""

I am not sure here...

IMHO lvs/pvs etc should not use prefix if there is --noheading, --separator and such script options.

But do we really want to change default output?

Except this default prefix change, ack.

Milan