Skip to contentSkip to conversation

dynamic debug: ehancements and cleanups

PATCH In review Jason Baron 4 participants 26 messages Dec 19, 2011 – Jan 24, 2012

Subsystems (7)
Networking Network drivers Documentation Library code +3 more subsystems
lore
Share & tools
lore
author
#1 lore raw

PATCH 1/16 dynamic_debug: fix whitespace complaints from scripts/cleanfile

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Style cleanups.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)
lib/dynamic_debug.c +9 -7
index dcdade3..e487d13 100644
@@ -187,7 +187,7 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 		if (!*buf)
 			break;	/* oh, it was trailing whitespace */
 
-		/* Run `end' over a word, either whitespace separated or quoted */
+		/* find `end' of word, whitespace separated or quoted */
 		if (*buf == '"' || *buf == '\'') {
 			int quote = *buf++;
 			for (end = buf ; *end && *end != quote ; end++)

@@ -199,8 +199,8 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 				;
 			BUG_ON(end == buf);
 		}
-		/* Here `buf' is the start of the word, `end' is one past the end */
 
+		/* `buf' is start of word, `end' is one past its end */
 		if (nwords == maxwords)
 			return -EINVAL;	/* ran out of words[] before bytes */
 		if (*end)

@@ -452,7 +452,8 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 					desc->function);
 	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
-		pos += snprintf(buf + pos, remaining(pos), "%d:", desc->lineno);
+		pos += snprintf(buf + pos, remaining(pos), "%d:",
+					desc->lineno);
 	if (pos - pos_after_tid)
 		pos += snprintf(buf + pos, remaining(pos), " ");
 	if (pos >= PREFIX_SIZE)

@@ -708,10 +709,11 @@ static const struct seq_operations ddebug_proc_seqops = {
 };
 
 /*
- * File_ops->open method for <debugfs>/dynamic_debug/control.  Does the seq_file
- * setup dance, and also creates an iterator to walk the _ddebugs.
- * Note that we create a seq_file always, even for O_WRONLY files
- * where it's not needed, as doing so simplifies the ->release method.
+ * File_ops->open method for <debugfs>/dynamic_debug/control.  Does
+ * the seq_file setup dance, and also creates an iterator to walk the
+ * _ddebugs.  Note that we create a seq_file always, even for O_WRONLY
+ * files where it's not needed, as doing so simplifies the ->release
+ * method.
  */
 static int ddebug_proc_open(struct inode *inode, struct file *file)
 {
Signature · 1 line
1.7.7.3
author
#4 lore raw

PATCH 4/16 dynamic_debug: change verbosity at runtime

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Allow changing dynamic_debug verbosity at run-time, to ease debugging of ddebug queries as you add them, improving usability.

at boot time: dynamic_debug.verbose=1
at runtime:

root@voyage:~# echo 1 > /sys/module/dynamic_debug/parameters/verbose
Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
lib/dynamic_debug.c +1 -0
index 416c079..8c88b89 100644
@@ -60,6 +60,7 @@ struct ddebug_iter {
 static DEFINE_MUTEX(ddebug_lock);
 static LIST_HEAD(ddebug_tables);
 static int verbose = 0;
+module_param(verbose, int, 0644);
 
 /* Return the last part of a pathname */
 static inline const char *basename(const char *path)
Signature · 1 line
1.7.7.3
author
#5 lore raw

PATCH 5/16 dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Replace strcpy with strlcpy, and add define for the size constant.

[[email protected]: Use DDEBUG_STRING_SIZE for overflow check]
Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
lib/dynamic_debug.c +5 -3
index 8c88b89..6fc8622 100644
@@ -525,14 +525,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
 
 #endif
 
-static __initdata char ddebug_setup_string[1024];
+#define DDEBUG_STRING_SIZE 1024
+static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
+
 static __init int ddebug_setup_query(char *str)
 {
-	if (strlen(str) >= 1024) {
+	if (strlen(str) >= DDEBUG_STRING_SIZE) {
 		pr_warn("ddebug boot param string too large\n");
 		return 0;
 	}
-	strcpy(ddebug_setup_string, str);
+	strlcpy(ddebug_setup_string, str, DDEBUG_STRING_SIZE);
 	return 1;
 }
Signature · 1 line
1.7.7.3
author
#8 lore raw

PATCH 6/16 dynamic_debug: pr_err() call should not depend upon verbosity

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Issue keyword/parsing errors even w/o verbose set; uncover otherwize mysterious non-functionality.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
lib/dynamic_debug.c +1 -2
index 6fc8622..d232025 100644
@@ -322,8 +322,7 @@ static int ddebug_parse_query(char *words[], int nwords,
 				query->last_lineno = query->first_lineno;
 			}
 		} else {
-			if (verbose)
-				pr_err("unknown keyword \"%s\"\n", words[i]);
+			pr_err("unknown keyword \"%s\"\n", words[i]);
 			return -EINVAL;
 		}
 	}
Signature · 1 line
1.7.7.3
author
#9 lore raw

PATCH 7/16 dynamic_debug: drop explicit !=NULL checks

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Convert 'if (x !=NULL)' checks into 'if (x)'.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)
lib/dynamic_debug.c +5 -6
index d232025..b199e09 100644
@@ -115,27 +115,26 @@ static void ddebug_change(const struct ddebug_query *query,
 	list_for_each_entry(dt, &ddebug_tables, link) {
 
 		/* match against the module name */
-		if (query->module != NULL &&
-		    strcmp(query->module, dt->mod_name))
+		if (query->module && strcmp(query->module, dt->mod_name))
 			continue;
 
 		for (i = 0 ; i < dt->num_ddebugs ; i++) {
 			struct _ddebug *dp = &dt->ddebugs[i];
 
 			/* match against the source filename */
-			if (query->filename != NULL &&
+			if (query->filename &&
 			    strcmp(query->filename, dp->filename) &&
 			    strcmp(query->filename, basename(dp->filename)))
 				continue;
 
 			/* match against the function */
-			if (query->function != NULL &&
+			if (query->function &&
 			    strcmp(query->function, dp->function))
 				continue;
 
 			/* match against the format */
-			if (query->format != NULL &&
-			    strstr(dp->format, query->format) == NULL)
+			if (query->format &&
+			    !strstr(dp->format, query->format))
 				continue;
 
 			/* match against the line number range */
Signature · 1 line
1.7.7.3
author
#10 lore raw

PATCH 8/16 dynamic_debug: describe_flags with '=[pmflt_]*'

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Change describe_flags() to emit '=[pmflt_]+' for current callsite flags, or just '=_' when they're disabled. Having '=' in output allows a more selective grep expression; in contrast '-' may appear in filenames, line-ranges, and format-strings. '=' also has better mnemonics, saying; "the current setting is equal to <flags>".

This allows grep "=_" <dbgfs>/dynamic_debug/control to see disabled callsites while avoiding the many occurrences of " = " seen in format strings.

Enlarge flagsbufs to handle additional flag char, and alter ddebug_parse_flags() to allow flags=0, so that user can turn off all debug flags via:

~# echo =_ > <dbgfs>/dynamic_debug/control

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 include/linux/dynamic_debug.h |    3 ++-
 lib/dynamic_debug.c           |   21 ++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)
include/linux/dynamic_debug.h +2 -1
index 29ea09a..fc39640 100644
@@ -21,7 +21,8 @@ struct _ddebug {
  	 * The bits here are changed dynamically when the user
 	 * writes commands to <debugfs>/dynamic_debug/control
 	 */
-#define _DPRINTK_FLAGS_PRINT   (1<<0)  /* printk() a message using the format */
+#define _DPRINTK_FLAGS_NONE	0
+#define _DPRINTK_FLAGS_PRINT	(1<<0) /* printk() a message using the format */
 #define _DPRINTK_FLAGS_INCL_MODNAME	(1<<1)
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
lib/dynamic_debug.c +10 -11
index b199e09..cde4dfe 100644
@@ -75,6 +75,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
 	{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
 	{ _DPRINTK_FLAGS_INCL_TID, 't' },
+	{ _DPRINTK_FLAGS_NONE, '_' },
 };
 
 /* format a string into buf[] which describes the _ddebug's flags */

@@ -84,12 +85,12 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
 	char *p = buf;
 	int i;
 
-	BUG_ON(maxlen < 4);
+	BUG_ON(maxlen < 6);
 	for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
 		if (dp->flags & opt_array[i].flag)
 			*p++ = opt_array[i].opt_char;
 	if (p == buf)
-		*p++ = '-';
+		*p++ = '_';
 	*p = '\0';
 
 	return buf;

@@ -108,7 +109,7 @@ static void ddebug_change(const struct ddebug_query *query,
 	struct ddebug_table *dt;
 	unsigned int newflags;
 	unsigned int nfound = 0;
-	char flagbuf[8];
+	char flagbuf[10];
 
 	/* search for matching ddebugs */
 	mutex_lock(&ddebug_lock);

@@ -152,7 +153,7 @@ static void ddebug_change(const struct ddebug_query *query,
 				continue;
 			dp->flags = newflags;
 			if (verbose)
-				pr_info("changed %s:%d [%s]%s %s\n",
+				pr_info("changed %s:%d [%s]%s =%s\n",
 					dp->filename, dp->lineno,
 					dt->mod_name, dp->function,
 					ddebug_describe_flags(dp, flagbuf,

@@ -370,8 +371,6 @@ static int ddebug_parse_flags(const char *str, unsigned int *flagsp,
 		if (i < 0)
 			return -EINVAL;
 	}
-	if (flags == 0)
-		return -EINVAL;
 	if (verbose)
 		pr_info("flags=0x%x\n", flags);
 

@@ -666,7 +665,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 {
 	struct ddebug_iter *iter = m->private;
 	struct _ddebug *dp = p;
-	char flagsbuf[8];
+	char flagsbuf[10];
 
 	if (verbose)
 		pr_info("called m=%p p=%p\n", m, p);

@@ -677,10 +676,10 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 		return 0;
 	}
 
-	seq_printf(m, "%s:%u [%s]%s %s \"",
-		   dp->filename, dp->lineno,
-		   iter->table->mod_name, dp->function,
-		   ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
+	seq_printf(m, "%s:%u [%s]%s =%s \"",
+		dp->filename, dp->lineno,
+		iter->table->mod_name, dp->function,
+		ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
 	seq_escape(m, dp->format, "\t\r\n\"");
 	seq_puts(m, "\"\n");
Signature · 1 line
1.7.7.3
author
#12 lore raw

PATCH 10/16 dynamic_debug: early return if _ddebug table is empty

In reply to Jason Baron

From: Jim Cromie <[email protected]>

If _ddebug table is empty (in a CONFIG_DYNAMIC_DEBUG build this shouldn't happen), then warn (error?) and return early. This skips empty table scan and parsing of setup-string, including the pr_info call noting the parse. By inspection, copy return-code handling from 1st ddebug_add_module() callsite to 2nd.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)
lib/dynamic_debug.c +20 -15
index 5a7bacc..4be55d8 100644
@@ -871,23 +871,28 @@ static int __init dynamic_debug_init(void)
 	int ret = 0;
 	int n = 0;
 
-	if (__start___verbose != __stop___verbose) {
-		iter = __start___verbose;
-		modname = iter->modname;
-		iter_start = iter;
-		for (; iter < __stop___verbose; iter++) {
-			if (strcmp(modname, iter->modname)) {
-				ret = ddebug_add_module(iter_start, n, modname);
-				if (ret)
-					goto out_free;
-				n = 0;
-				modname = iter->modname;
-				iter_start = iter;
-			}
-			n++;
+	if (__start___verbose == __stop___verbose) {
+		pr_warn("_ddebug table is empty in a "
+			"CONFIG_DYNAMIC_DEBUG build");
+		return 1;
+	}
+	iter = __start___verbose;
+	modname = iter->modname;
+	iter_start = iter;
+	for (; iter < __stop___verbose; iter++) {
+		if (strcmp(modname, iter->modname)) {
+			ret = ddebug_add_module(iter_start, n, modname);
+			if (ret)
+				goto out_free;
+			n = 0;
+			modname = iter->modname;
+			iter_start = iter;
 		}
-		ret = ddebug_add_module(iter_start, n, modname);
+		n++;
 	}
+	ret = ddebug_add_module(iter_start, n, modname);
+	if (ret)
+		goto out_free;
 
 	/* ddebug_query boot param got passed -> set it up */
 	if (ddebug_setup_string[0] != '\0') {
Signature · 1 line
1.7.7.3
author
#11 lore raw

PATCH 9/16 dynamic_debug: tighten up error checking on debug queries

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Issue error when a match-spec is given multiple times in a rule. Previous code kept last one, but was silent about it. Docs imply only one is allowed by saying match-specs are ANDed together, given that module M cannot match both A and B. Also error when last_line < 1st_line.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   39 +++++++++++++++++++++++++++++++++------
 1 files changed, 33 insertions(+), 6 deletions(-)
lib/dynamic_debug.c +33 -6
index cde4dfe..5a7bacc 100644
@@ -276,6 +276,19 @@ static char *unescape(char *str)
 	return str;
 }
 
+static int check_set(const char **dest, char *src, char *name)
+{
+	int rc = 0;
+
+	if (*dest) {
+		rc = -EINVAL;
+		pr_err("match-spec:%s val:%s overridden by %s",
+			name, *dest, src);
+	}
+	*dest = src;
+	return rc;
+}
+
 /*
  * Parse words[] as a ddebug query specification, which is a series
  * of (keyword, value) pairs chosen from these possibilities:

@@ -287,11 +300,15 @@ static char *unescape(char *str)
  * format <escaped-string-to-find-in-format>
  * line <lineno>
  * line <first-lineno>-<last-lineno> // where either may be empty
+ *
+ * Only 1 of each type is allowed.
+ * Returns 0 on success, <0 on error.
  */
 static int ddebug_parse_query(char *words[], int nwords,
 			       struct ddebug_query *query)
 {
 	unsigned int i;
+	int rc;
 
 	/* check we have an even number of words */
 	if (nwords % 2 != 0)

@@ -300,24 +317,32 @@ static int ddebug_parse_query(char *words[], int nwords,
 
 	for (i = 0 ; i < nwords ; i += 2) {
 		if (!strcmp(words[i], "func"))
-			query->function = words[i+1];
+			rc = check_set(&query->function, words[i+1], "func");
 		else if (!strcmp(words[i], "file"))
-			query->filename = words[i+1];
+			rc = check_set(&query->filename, words[i+1], "file");
 		else if (!strcmp(words[i], "module"))
-			query->module = words[i+1];
+			rc = check_set(&query->module, words[i+1], "module");
 		else if (!strcmp(words[i], "format"))
-			query->format = unescape(words[i+1]);
+			rc = check_set(&query->format, unescape(words[i+1]),
+				"format");
 		else if (!strcmp(words[i], "line")) {
 			char *first = words[i+1];
 			char *last = strchr(first, '-');
+			if (query->first_lineno || query->last_lineno) {
+				pr_err("match-spec:line given 2 times\n");
+				return -EINVAL;
+			}
 			if (last)
 				*last++ = '\0';
 			if (parse_lineno(first, &query->first_lineno) < 0)
 				return -EINVAL;
-			if (last != NULL) {
+			if (last) {
 				/* range <first>-<last> */
-				if (parse_lineno(last, &query->last_lineno) < 0)
+				if (parse_lineno(last, &query->last_lineno)
+				    < query->first_lineno) {
+					pr_err("last-line < 1st-line\n");
 					return -EINVAL;
+				}
 			} else {
 				query->last_lineno = query->first_lineno;
 			}

@@ -325,6 +350,8 @@ static int ddebug_parse_query(char *words[], int nwords,
 			pr_err("unknown keyword \"%s\"\n", words[i]);
 			return -EINVAL;
 		}
+		if (rc)
+			return rc;
 	}
 
 	if (verbose)
Signature · 1 line
1.7.7.3
author
#14 lore raw

PATCH 12/16 dynamic_debug: chop off comments in ddebug_tokenize

In reply to Jason Baron

From: Jim Cromie <[email protected]>

If a token begins with #, the remainder of query string is a comment, so drop it. Doing it here avoids '#' in quoted strings.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
lib/dynamic_debug.c +2 -0
index 4be55d8..86a9abb 100644
@@ -183,6 +183,8 @@ static int ddebug_tokenize(char *buf, char *words[], int maxwords)
 		buf = skip_spaces(buf);
 		if (!*buf)
 			break;	/* oh, it was trailing whitespace */
+		if (*buf == '#')
+			break;	/* token starts comment, skip rest of line */
 
 		/* find `end' of word, whitespace separated or quoted */
 		if (*buf == '"' || *buf == '\'') {
Signature · 1 line
1.7.7.3
author
#15 lore raw

PATCH 13/16 dynamic_debug: enlarge command/query write buffer

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Current query write buffer is 256 bytes, on stack. In comparison, the ddebug_query boot-arg is 1024. Allocate the buffer off heap, and enlarge it to 4096 bytes, big enough for ~100 queries (at 40 bytes each), and error out if not. This makes it play nicely with large query sets (to be added later). The buffer should be enough for most uses, and others should probably be split into subsets.

[[email protected]: changed USER_BUF_PAGE from 4095 -> 4096 ]
Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)
lib/dynamic_debug.c +12 -4
index 86a9abb..d8773dc 100644
@@ -570,24 +570,32 @@ __setup("ddebug_query=", ddebug_setup_query);
  * File_ops->write method for <debugfs>/dynamic_debug/conrol.  Gathers the
  * command text from userspace, parses and executes it.
  */
+#define USER_BUF_PAGE 4096
 static ssize_t ddebug_proc_write(struct file *file, const char __user *ubuf,
 				  size_t len, loff_t *offp)
 {
-	char tmpbuf[256];
+	char *tmpbuf;
 	int ret;
 
 	if (len == 0)
 		return 0;
-	/* we don't check *offp -- multiple writes() are allowed */
-	if (len > sizeof(tmpbuf)-1)
+	if (len > USER_BUF_PAGE - 1) {
+		pr_warn("expected <%d bytes into control\n", USER_BUF_PAGE);
 		return -E2BIG;
-	if (copy_from_user(tmpbuf, ubuf, len))
+	}
+	tmpbuf = kmalloc(len + 1, GFP_KERNEL);
+	if (!tmpbuf)
+		return -ENOMEM;
+	if (copy_from_user(tmpbuf, ubuf, len)) {
+		kfree(tmpbuf);
 		return -EFAULT;
+	}
 	tmpbuf[len] = '\0';
 	if (verbose)
 		pr_info("read %d bytes from userspace\n", (int)len);
 
 	ret = ddebug_exec_query(tmpbuf);
+	kfree(tmpbuf);
 	if (ret)
 		return ret;
Signature · 1 line
1.7.7.3
author
#16 lore raw

PATCH 14/16 dynamic_debug: add trim_prefix() to provide source-root relative paths

In reply to Jason Baron

From: Jim Cromie <[email protected]>

trim_prefix(path) skips past the absolute source path root, and returns the pointer to the relative path from there. It is used to shorten the displayed path in $DBGMT/dynamic_debug/control via ddebug_proc_show(), and in ddebug_change() to allow relative filenames to be used in applied queries. For example:

~# echo file kernel/freezer.c +p > $DBGMT/dynamic_debug/control

kernel/freezer.c:128 [freezer]cancel_freezing p " clean up: %s\012"

trim_prefix(path) insures common prefix before trimming it, so out-of-tree module paths are shown as full absolute paths.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 Documentation/dynamic-debug-howto.txt |    7 ++++---
 lib/dynamic_debug.c                   |   18 +++++++++++++++---
 2 files changed, 19 insertions(+), 6 deletions(-)
Documentation/dynamic-debug-howto.txt +4 -3
index f959909..378b5d1 100644
@@ -144,11 +144,12 @@ func
     func svc_tcp_accept
 
 file
-    The given string is compared against either the full
-    pathname or the basename of the source file of each
-    callsite.  Examples:
+    The given string is compared against either the full pathname, the
+    src-root relative pathname, or the basename of the source file of
+    each callsite.  Examples:
 
     file svcsock.c
+    file kernel/freezer.c
     file /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c
 
 module
lib/dynamic_debug.c +15 -3
index d8773dc..a5508a1 100644
@@ -69,6 +69,17 @@ static inline const char *basename(const char *path)
 	return tail ? tail+1 : path;
 }
 
+/* Return the path relative to source root */
+static inline const char *trim_prefix(const char *path)
+{
+	int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
+
+	if (strncmp(path, __FILE__, skip))
+		skip = 0; /* prefix mismatch, don't skip */
+
+	return path + skip;
+}
+
 static struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINT, 'p' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },

@@ -125,7 +136,8 @@ static void ddebug_change(const struct ddebug_query *query,
 			/* match against the source filename */
 			if (query->filename &&
 			    strcmp(query->filename, dp->filename) &&
-			    strcmp(query->filename, basename(dp->filename)))
+			    strcmp(query->filename, basename(dp->filename)) &&
+			    strcmp(query->filename, trim_prefix(dp->filename)))
 				continue;
 
 			/* match against the function */

@@ -154,7 +166,7 @@ static void ddebug_change(const struct ddebug_query *query,
 			dp->flags = newflags;
 			if (verbose)
 				pr_info("changed %s:%d [%s]%s =%s\n",
-					dp->filename, dp->lineno,
+					trim_prefix(dp->filename), dp->lineno,
 					dt->mod_name, dp->function,
 					ddebug_describe_flags(dp, flagbuf,
 							sizeof(flagbuf)));

@@ -714,7 +726,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	}
 
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
-		dp->filename, dp->lineno,
+		trim_prefix(dp->filename), dp->lineno,
 		iter->table->mod_name, dp->function,
 		ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
 	seq_escape(m, dp->format, "\t\r\n\"");
Signature · 1 line
1.7.7.3
author
#19 lore raw

PATCH 15/16 dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(), for reuse later. Also change the printed labels: file, func to agree with the query-spec keywords accepted in the control file. Pass "" when string is null, to avoid "(null)" output from sprintf. For format print, use precision to skip last char, assuming its '\n', no great harm if not, its a debug msg.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 lib/dynamic_debug.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)
lib/dynamic_debug.c +17 -8
index a5508a1..93fc5d5 100644
@@ -107,6 +107,22 @@ static char *ddebug_describe_flags(struct _ddebug *dp, char *buf,
 	return buf;
 }
 
+#define vpr_info_dq(q, msg)						\
+do {									\
+	if (verbose)							\
+		/* trim last char off format print */			\
+		pr_info("%s: func=\"%s\" file=\"%s\" "			\
+			"module=\"%s\" format=\"%.*s\" "		\
+			"lineno=%u-%u",					\
+			msg,						\
+			q->function ? q->function : "",			\
+			q->filename ? q->filename : "",			\
+			q->module ? q->module : "",			\
+			(int)(q->format ? strlen(q->format) - 1 : 0),	\
+			q->format ? q->format : "",			\
+			q->first_lineno, q->last_lineno);		\
+} while (0)
+
 /*
  * Search the tables for _ddebug's which match the given
  * `query' and apply the `flags' and `mask' to them.  Tells

@@ -367,14 +383,7 @@ static int ddebug_parse_query(char *words[], int nwords,
 		if (rc)
 			return rc;
 	}
-
-	if (verbose)
-		pr_info("q->function=\"%s\" q->filename=\"%s\" "
-			"q->module=\"%s\" q->format=\"%s\" q->lineno=%u-%u\n",
-			query->function, query->filename,
-			query->module, query->format, query->first_lineno,
-			query->last_lineno);
-
+	vpr_info_dq(query, "parsed");
 	return 0;
 }
Signature · 1 line
1.7.7.3
author
#25 lore raw

PATCH 16/16 dynamic_debug: process multiple debug-queries on a line

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Insert ddebug_exec_queries() in place of ddebug_exec_query(). It splits the query string on [;\n], and calls ddebug_exec_query() on each. All queries are processed independent of errors, allowing a query to fail, for example when a module is not installed. Empty lines and comments are skipped. Errors are counted, and the last error seen (negative) or the number of callsites found (0 or positive) is returned. Return code checks are altered accordingly.

With this, multiple queries can be given in ddebug_query, allowing more selective enabling of callsites. As a side effect, a set of commands can be batched in:

cat cmd-file > $DBGMT/dynamic_debug/control

We dont want a ddebug_query syntax error to kill the dynamic debug facility, so dynamic_debug_init() zeros ddebug_exec_queries()'s return code after logging the appropriate message, so that ddebug tables are preserved and $DBGMT/dynamic_debug/control file is created. This would be appropriate even without accepting multiple queries.

This patch also alters ddebug_change() to return number of callsites matched (which typically is the same as number of callsites changed). ddebug_exec_query() also returns the number found, or a negative value if theres a parse error on the query.

Splitting on [;\n] prevents their use in format-specs, but selecting callsites on punctuation is brittle anyway, meaningful and selective substrings are more typical.

Note: splitting queries on ';' before handling trailing #comments means that a ';' also terminates a comment, and text after the ';' is treated as another query. This trailing query will almost certainly result in a parse error and thus have no effect other than the error message. The double corner case with unexpected results is:

ddebug_query="func foo +p # enable foo ; +p"

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 Documentation/dynamic-debug-howto.txt |   23 ++++-------
 lib/dynamic_debug.c                   |   73 ++++++++++++++++++++++++++-------
 2 files changed, 66 insertions(+), 30 deletions(-)
Documentation/dynamic-debug-howto.txt +8 -15
index 378b5d1..74e6c77 100644
@@ -12,7 +12,7 @@ dynamically enabled per-callsite.
 Dynamic debug has even more useful features:
 
  * Simple query language allows turning on and off debugging statements by
-   matching any combination of:
+   matching any combination of 0 or 1 of:
 
    - source filename
    - function name

@@ -79,31 +79,24 @@ Command Language Reference
 ==========================
 
 At the lexical level, a command comprises a sequence of words separated
-by whitespace characters.  Note that newlines are treated as word
-separators and do *not* end a command or allow multiple commands to
-be done together.  So these are all equivalent:
+by spaces or tabs.  So these are all equivalent:
 
 nullarbor:~ # echo -c 'file svcsock.c line 1603 +p' >
 				<debugfs>/dynamic_debug/control
 nullarbor:~ # echo -c '  file   svcsock.c     line  1603 +p  ' >
 				<debugfs>/dynamic_debug/control
-nullarbor:~ # echo -c 'file svcsock.c\nline 1603 +p' >
-				<debugfs>/dynamic_debug/control
 nullarbor:~ # echo -n 'file svcsock.c line 1603 +p' >
 				<debugfs>/dynamic_debug/control
 
-Commands are bounded by a write() system call.  If you want to do
-multiple commands you need to do a separate "echo" for each, like:
+Command submissions are bounded by a write() system call.
+Multiple commands can be written together, separated by ';' or '\n'.
 
-nullarbor:~ # echo 'file svcsock.c line 1603 +p' > /proc/dprintk ;\
-> echo 'file svcsock.c line 1563 +p' > /proc/dprintk
+  ~# echo "func pnpacpi_get_resources +p; func pnp_assign_mem +p" \
+     > <debugfs>/dynamic_debug/control
 
-or even like:
+If your query set is big, you can batch them too:
 
-nullarbor:~ # (
-> echo 'file svcsock.c line 1603 +p' ;\
-> echo 'file svcsock.c line 1563 +p' ;\
-> ) > /proc/dprintk
+  ~# cat query-batch-file > <debugfs>/dynamic_debug/control
 
 At the syntactical level, a command comprises a sequence of match
 specifications, followed by a flags change specification.
lib/dynamic_debug.c +58 -15

114 lines not on this page · raw message

Signature · 1 line
1.7.7.3
author
#13 lore raw

PATCH 11/16 dynamic_debug: reduce lineno field to a saner 18 bits

In reply to Jason Baron

From: Jim Cromie <[email protected]>

lineno:24 allows files with 4 million lines, an insane file-size, even for never-to-get-in-tree machine generated code. Reduce this to 18 bits, which still allows 256k lines. This is still insanely big, but its not raving mad.

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 include/linux/dynamic_debug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/dynamic_debug.h +1 -1
index fc39640..7e3c53a 100644
@@ -15,7 +15,7 @@ struct _ddebug {
 	const char *function;
 	const char *filename;
 	const char *format;
-	unsigned int lineno:24;
+	unsigned int lineno:18;
 	/*
  	 * The flags field controls the behaviour at the callsite.
  	 * The bits here are changed dynamically when the user
Signature · 1 line
1.7.7.3

Jason Baron

author
#0 lore raw

cover dynamic debug: ehancements and cleanups

Hi,

Mostly cleanups and minor feature ehancements for dynamic debug. I haven't included the module parameter feature, since its implementation is still being worked out, but I wanted to get some of Jim's patches included, since I've got so many. These have already been through several review iterations. Patches are against Linus's latest tree.

Thanks,

-Jason

Jim Cromie (16):
dynamic_debug: fix whitespace complaints from scripts/cleanfile
dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT
dynamic_debug: make dynamic-debug supersede DEBUG ccflag
dynamic_debug: change verbosity at runtime
dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()
dynamic_debug: pr_err() call should not depend upon verbosity
dynamic_debug: drop explicit !=NULL checks
dynamic_debug: describe_flags with '=[pmflt_]*'
dynamic_debug: tighten up error checking on debug queries
dynamic_debug: early return if _ddebug table is empty
dynamic_debug: reduce lineno field to a saner 18 bits
dynamic_debug: chop off comments in ddebug_tokenize
dynamic_debug: enlarge command/query write buffer
dynamic_debug: add trim_prefix() to provide source-root relative paths
dynamic_debug: factor vpr_info_dq out of ddebug_parse_query
dynamic_debug: process multiple debug-queries on a line

 Documentation/dynamic-debug-howto.txt |   30 ++---
 include/linux/device.h                |    8 +-
 include/linux/dynamic_debug.h         |   19 ++-
 include/linux/netdevice.h             |    8 +-
 include/linux/printk.h                |    8 +-
 lib/dynamic_debug.c                   |  270 +++++++++++++++++++++++----------
 6 files changed, 222 insertions(+), 121 deletions(-)
Signature · 1 line
1.7.7.3
author
#3 lore raw

PATCH 3/16 dynamic_debug: make dynamic-debug supersede DEBUG ccflag

From: Jim Cromie <[email protected]>

If CONFIG_DYNAMIC_DEBUG is defined, honor it over DEBUG, so that pr_debug()s are controllable, instead of always-on. When DEBUG is also defined, change _DPRINTK_FLAGS_DEFAULT to enable printing by default.

Also adding _DPRINTK_FLAGS_INCL_MODNAME would be nice, but there are numerous cases of pr_debug(NAME ": ...), which would result in double printing of module-name. So defer this until things settle.

Cc David Miller <[email protected]>
Cc Joe Perches <[email protected]>
Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 include/linux/device.h        |    8 ++++----
 include/linux/dynamic_debug.h |    4 ++++
 include/linux/netdevice.h     |    8 ++++----
 include/linux/printk.h        |    8 ++++----
 4 files changed, 16 insertions(+), 12 deletions(-)
include/linux/device.h +4 -4
index 3136ede..c9468c1 100644
@@ -872,14 +872,14 @@ int _dev_info(const struct device *dev, const char *fmt, ...)
 
 #define dev_info(dev, fmt, arg...) _dev_info(dev, fmt, ##arg)
 
-#if defined(DEBUG)
-#define dev_dbg(dev, format, arg...)		\
-	dev_printk(KERN_DEBUG, dev, format, ##arg)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
 #define dev_dbg(dev, format, ...)		     \
 do {						     \
 	dynamic_dev_dbg(dev, format, ##__VA_ARGS__); \
 } while (0)
+#elif defined(DEBUG)
+#define dev_dbg(dev, format, arg...)		\
+	dev_printk(KERN_DEBUG, dev, format, ##arg)
 #else
 #define dev_dbg(dev, format, arg...)				\
 ({								\
include/linux/dynamic_debug.h +4 -0
index f71a6b04..29ea09a 100644
@@ -26,7 +26,11 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
 #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
+#if defined DEBUG
+#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
+#else
 #define _DPRINTK_FLAGS_DEFAULT 0
+#endif
 	unsigned int flags:8;
 } __attribute__((aligned(8)));
include/linux/netdevice.h +4 -4
index a82ad4d..9a222e7 100644
@@ -2645,14 +2645,14 @@ int netdev_info(const struct net_device *dev, const char *format, ...);
 #define MODULE_ALIAS_NETDEV(device) \
 	MODULE_ALIAS("netdev-" device)
 
-#if defined(DEBUG)
-#define netdev_dbg(__dev, format, args...)			\
-	netdev_printk(KERN_DEBUG, __dev, format, ##args)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
 #define netdev_dbg(__dev, format, args...)			\
 do {								\
 	dynamic_netdev_dbg(__dev, format, ##args);		\
 } while (0)
+#elif defined(DEBUG)
+#define netdev_dbg(__dev, format, args...)			\
+	netdev_printk(KERN_DEBUG, __dev, format, ##args)
 #else
 #define netdev_dbg(__dev, format, args...)			\
 ({								\
include/linux/printk.h +4 -4
index f0e22f7..f9abd93 100644
@@ -180,13 +180,13 @@ extern void dump_stack(void) __cold;
 #endif
 
 /* If you are writing a driver, please use dev_dbg instead */
-#if defined(DEBUG)
-#define pr_debug(fmt, ...) \
-	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
-#elif defined(CONFIG_DYNAMIC_DEBUG)
+#if defined(CONFIG_DYNAMIC_DEBUG)
 /* dynamic_pr_debug() uses pr_fmt() internally so we don't need it here */
 #define pr_debug(fmt, ...) \
 	dynamic_pr_debug(fmt, ##__VA_ARGS__)
+#elif defined(DEBUG)
+#define pr_debug(fmt, ...) \
+	printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
 #else
 #define pr_debug(fmt, ...) \
 	no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
Signature · 1 line
1.7.7.3
author
#2 lore raw

PATCH 2/16 dynamic_debug: drop enabled field from struct _ddebug, use _DPRINTK_FLAGS_PRINT

In reply to Jason Baron

From: Jim Cromie <[email protected]>

Currently any enabled dynamic-debug flag on a pr_debug callsite will enable printing, even if _DPRINTK_FLAGS_PRINT is off. Checking print flag directly allows "-p" to disable callsites without fussing with other flags, so the following disables everything, without altering flags user may have set:

echo -p > $DBGFS/dynamic_debug/control

Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>
---
 include/linux/dynamic_debug.h |   10 ++++------
 lib/dynamic_debug.c           |    4 ----
 2 files changed, 4 insertions(+), 10 deletions(-)
include/linux/dynamic_debug.h +4 -6
index 0564e3c..f71a6b04 100644
@@ -28,7 +28,6 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
 #define _DPRINTK_FLAGS_DEFAULT 0
 	unsigned int flags:8;
-	char enabled;
 } __attribute__((aligned(8)));
 
 

@@ -62,21 +61,20 @@ int __dynamic_netdev_dbg(struct _ddebug *descriptor,
 		.format = (fmt),				\
 		.lineno = __LINE__,				\
 		.flags =  _DPRINTK_FLAGS_DEFAULT,		\
-		.enabled = false,				\
 	}
 
 #define dynamic_pr_debug(fmt, ...)				\
 do {								\
 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
-	if (unlikely(descriptor.enabled))			\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
 		__dynamic_pr_debug(&descriptor, pr_fmt(fmt),	\
 				   ##__VA_ARGS__);		\
 } while (0)
 
 #define dynamic_dev_dbg(dev, fmt, ...)				\
 do {								\
-	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);	\
-	if (unlikely(descriptor.enabled))			\
+	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
 		__dynamic_dev_dbg(&descriptor, dev, fmt,	\
 				  ##__VA_ARGS__);		\
 } while (0)

@@ -84,7 +82,7 @@ do {								\
 #define dynamic_netdev_dbg(dev, fmt, ...)			\
 do {								\
 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);		\
-	if (unlikely(descriptor.enabled))			\
+	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT))	\
 		__dynamic_netdev_dbg(&descriptor, dev, fmt,	\
 				     ##__VA_ARGS__);		\
 } while (0)
lib/dynamic_debug.c +0 -4
index e487d13..416c079 100644
@@ -151,10 +151,6 @@ static void ddebug_change(const struct ddebug_query *query,
 			if (newflags == dp->flags)
 				continue;
 			dp->flags = newflags;
-			if (newflags)
-				dp->enabled = 1;
-			else
-				dp->enabled = 0;
 			if (verbose)
 				pr_info("changed %s:%d [%s]%s %s\n",
 					dp->filename, dp->lineno,
Signature · 1 line
1.7.7.3
#6 lore raw

Re: [PATCH 05/16] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

In reply to Jason Baron
Quoted 25 lines · Jason Baron

From: Jim Cromie <[email protected]>

Replace strcpy with strlcpy, and add define for the size constant.

[[email protected]: Use DDEBUG_STRING_SIZE for overflow check]
Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>

---
 lib/dynamic_debug.c |    8 +++++---

 1 files changed, 5 insertions(+), 3 deletions(-)
lib/dynamic_debug.c +3 -1
index 8c88b89..6fc8622 100644
@@ -525,14 +525,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
 
  #endif
 
-static __initdata char ddebug_setup_string[1024];
+#define DDEBUG_STRING_SIZE 1024
+static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
+
  static __init int ddebug_setup_query(char *str)

lib/dynamic_debug.c EXPORT_SYMBOL(__dynamic_netdev_dbg); · line 539

Quoted 10 lines
lib/dynamic_debug.c +2 -2
  {
-       if (strlen(str) >= 1024) {
+       if (strlen(str) >= DDEBUG_STRING_SIZE) {
                pr_warn("ddebug boot param string too large\n");
                return 0;
        }
-       strcpy(ddebug_setup_string, str);
+       strlcpy(ddebug_setup_string, str, DDEBUG_STRING_SIZE);
        return 1;
  }

Sorry, but it's not clear to me why it make sense to introduce the DDEBUG_STRING_SIZE macro instead of using ARRAY_SIZE().

Bart.

#17 lore raw

Re: [PATCH 14/16] dynamic_debug: add trim_prefix() to provide source-root relative paths

In reply to Jason Baron

lib/dynamic_debug.c static inline const char *basename(const char *path) · line 81

Quoted 10 lines · Jason Baron
lib/dynamic_debug.c +10 -0
+/* Return the path relative to source root */
+static inline const char *trim_prefix(const char *path)
+{
+       int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
+
+       if (strncmp(path, __FILE__, skip))
+               skip = 0; /* prefix mismatch, don't skip */
+
+       return path + skip;
+}

Isn't the preferred style to write if (strncmp(...) != 0) ..., i.e. to mention "!= 0" explicitly instead of leaving it out when comparing strings ?

Bart.

#20 lore raw

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

In reply to Jason Baron
Quoted 8 lines · Jason Baron

From: Jim Cromie <[email protected]>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(), for reuse later.  Also change the printed labels: file, func to agree with the query-spec keywords accepted in the control file.  Pass "" when string is null, to avoid "(null)" output from sprintf.  For format print, use precision to skip last char, assuming its '\n', no great harm if not, its a debug msg.

pr_info_dq() or vpr_info_dq() ?

Bart.

maintainer
#21 lore raw

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

Quoted 10 lines · Bart Van Assche
Quoted 8 lines · Jason Baron

From: Jim Cromie <[email protected]>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(), for reuse later.  Also change the printed labels: file, func to agree with the query-spec keywords accepted in the control file.  Pass "" when string is null, to avoid "(null)" output from sprintf.  For format print, use precision to skip last char, assuming its '\n', no great harm if not, its a debug msg.

Quoted 3 lines

pr_info_dq() or vpr_info_dq() ?

Bart.

I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part

#22 lore raw

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

Quoted 11 lines · Jim Cromie
Quoted 10 lines · Bart Van Assche
Quoted 8 lines · Jason Baron

From: Jim Cromie <[email protected]>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(), for reuse later.  Also change the printed labels: file, func to agree with the query-spec keywords accepted in the control file.  Pass "" when string is null, to avoid "(null)" output from sprintf.  For format print, use precision to skip last char, assuming its '\n', no great harm if not, its a debug msg.

Quoted 3 lines
Quoted 1 line

pr_info_dq() or vpr_info_dq() ?

I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part

In case my reply was a little too terse: isn't there a letter "v" missing in the commit message ?

Bart.

author
#7 lore raw

Re: [PATCH 05/16] dynamic_debug: replace strcpy with strlcpy, in ddebug_setup_query()

In reply to Bart Van Assche
Quoted 40 lines · Bart Van Assche
Quoted 25 lines · Jason Baron

From: Jim Cromie <[email protected]>

Replace strcpy with strlcpy, and add define for the size constant.

[[email protected]: Use DDEBUG_STRING_SIZE for overflow check]
Signed-off-by Jim Cromie <[email protected]>
Signed-off-by Jason Baron <[email protected]>

---
 lib/dynamic_debug.c |    8 +++++---

 1 files changed, 5 insertions(+), 3 deletions(-)
lib/dynamic_debug.c +3 -1
index 8c88b89..6fc8622 100644
@@ -525,14 +525,16 @@ EXPORT_SYMBOL(__dynamic_netdev_dbg);
 
  #endif
 
-static __initdata char ddebug_setup_string[1024];
+#define DDEBUG_STRING_SIZE 1024
+static __initdata char ddebug_setup_string[DDEBUG_STRING_SIZE];
+
  static __init int ddebug_setup_query(char *str)

lib/dynamic_debug.c EXPORT_SYMBOL(__dynamic_netdev_dbg); · line 539

Quoted 10 lines
lib/dynamic_debug.c +2 -2
  {
-       if (strlen(str) >= 1024) {
+       if (strlen(str) >= DDEBUG_STRING_SIZE) {
                pr_warn("ddebug boot param string too large\n");
                return 0;
        }
-       strcpy(ddebug_setup_string, str);
+       strlcpy(ddebug_setup_string, str, DDEBUG_STRING_SIZE);
        return 1;
  }

Sorry, but it's not clear to me why it make sense to introduce the DDEBUG_STRING_SIZE macro instead of using ARRAY_SIZE().

Quoted 1 line

Bart.

Greg,

The change suggested here by Bart isn't a functional change. If you think it warrants a re-post, let me know, but I am fine with it as-is.

Thanks,

-Jason

author
#18 lore raw

Re: [PATCH 14/16] dynamic_debug: add trim_prefix() to provide source-root relative paths

In reply to Bart Van Assche
Quoted 16 lines · Bart Van Assche

lib/dynamic_debug.c static inline const char *basename(const char *path) · line 81

Quoted 10 lines · Jason Baron
lib/dynamic_debug.c +10 -0
+/* Return the path relative to source root */
+static inline const char *trim_prefix(const char *path)
+{
+       int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
+
+       if (strncmp(path, __FILE__, skip))
+               skip = 0; /* prefix mismatch, don't skip */
+
+       return path + skip;
+}

Isn't the preferred style to write if (strncmp(...) != 0) ..., i.e. to mention "!= 0" explicitly instead of leaving it out when comparing strings ?

Quoted 1 line

Bart.

Greg,

Again, no functional change here, just a style thing. Let me know if you want a re-post. I am fine with it as-is.

Thanks,

-Jason

author
#23 lore raw

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

In reply to Bart Van Assche
Quoted 12 lines · Bart Van Assche
Quoted 11 lines · Jim Cromie
Quoted 10 lines · Bart Van Assche
Quoted 8 lines · Jason Baron

From: Jim Cromie <[email protected]>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(), for reuse later.  Also change the printed labels: file, func to agree with the query-spec keywords accepted in the control file.  Pass "" when string is null, to avoid "(null)" output from sprintf.  For format print, use precision to skip last char, assuming its '\n', no great harm if not, its a debug msg.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

pr_info_dq() or vpr_info_dq() ?

I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part

In case my reply was a little too terse: isn't there a letter "v" missing in the commit message ?

Bart.

Thanks for catching this.

Greg, if you want to s/pr_info_dq/vpr_info_dq, in the commit msg, this one is all set.

Thanks,

-Jason

#24 lore raw

Re: [PATCH 15/16] dynamic_debug: factor vpr_info_dq out of ddebug_parse_query

Quoted 20 lines · Jason Baron
Quoted 19 lines · Bart Van Assche
Quoted 11 lines · Jim Cromie
Quoted 10 lines · Bart Van Assche
Quoted 8 lines · Jason Baron

From: Jim Cromie <[email protected]>

Factor pr_info(query) out of ddebug_parse_query, into pr_info_dq(), for reuse later.  Also change the printed labels: file, func to agree with the query-spec keywords accepted in the control file.  Pass "" when string is null, to avoid "(null)" output from sprintf.  For format print, use precision to skip last char, assuming its '\n', no great harm if not, its a debug msg.

Quoted 3 lines
Quoted 1 line

pr_info_dq() or vpr_info_dq() ?

I chose vpr_info_dq, with leading v to hint at the "if (verbose)" part

In case my reply was a little too terse: isn't there a letter "v" missing in the commit message ?

Quoted 6 lines
Quoted 1 line

Bart.

Thanks for catching this.

Greg, if you want to s/pr_info_dq/vpr_info_dq, in the commit msg, this one is all set.

Now done, thanks.

greg k-h