kernelnewbies.kernelnewbies.org archive mirror
 help / color / mirror / Atom feed
From: jim.cromie@gmail.com
To: Varun Iyer <varun_iyer@posteo.net>
Cc: Greg KH <greg@kroah.com>,
	kernelnewbies <kernelnewbies@kernelnewbies.org>
Subject: Re: is there a macro symbol for strlen($LINUX_SOURCE_PATH)
Date: Thu, 5 Sep 2019 08:31:55 -0600	[thread overview]
Message-ID: <CAJfuBxwChECpA8j+GWdyy+wAtydFybjDLSCQjcf7C-L0Oa2iQA@mail.gmail.com> (raw)
In-Reply-To: <20190904221817.gismdbom2rkdyfx6@varun-laptop>

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

On Wed, Sep 4, 2019 at 4:14 PM Varun Iyer <varun_iyer@posteo.net> wrote:
>
> On 19/09/04 03:42PM, jim.cromie@gmail.com wrote:
> > in the kernel, there are a lot of usages of __FILE__
> >
> > this means that there are a lot of copies of fully rooted paths,
> > including all the varying ~/src/linux prefixes used by individual builders.
>
> On gcc, `__FILE__` is the path of the file relative to the directory that the
> build is run from.
>
> Try something like
>
> #include <stdio.h>
> #define testing() printf("%s\n", __FILE__)
> int main() { testing(); }
>
> and compile it from different directories to replicate this behaviour.
>

whoa.  how long have I been out ?

per https://www.cprogramming.com/reference/preprocessor/__FILE__.html
__FILE__ is a preprocessor macro that expands to full path to the current file.

and thanks for that simple demo.
I shoulda done that myself, but I had no concept that such a change
might happen.

and its not the whole story.
when I rebuild from .. the __FILE__ value changes

[jimc@frodo play]$ make gcc/file.o
cc    -c -o gcc/file.o gcc/file.c
[jimc@frodo play]$ make gcc/file
cc   gcc/file.o   -o gcc/file
[jimc@frodo play]$ gcc/file
gcc/file.c

so its build-path relative (which u pretty much said), but I doubt its
that simple either.
I presume it is also controllable / customizable using one or more
make variables or techniques.
linux/Makefile's  $srctree variable looks to be involved
Perhaps this is (one more reason) why (shell-descent) recursive makes
are less than desirable ?

In any case I added a printk("__FILE__: %s\n", __FILE__)
to see for myself, and you are correct.

So I feel compelled to offer a fix for dynamic_debug, attached.
hopefully it explains adequately, I have some doubts..

maybe this should go to LKML now,
but I guess Id prefer to make my obvious thinkos less publicly.
Im happy to bikeshed the commit-msg or code.

[-- Attachment #2: 0001-dynamic_debug-drop-obsolete-trim_prefix.patch --]
[-- Type: text/x-patch, Size: 3624 bytes --]

From c7b8041928565786fc58fd20e80b278cc877bef4 Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Thu, 5 Sep 2019 00:25:34 -0600
Subject: [PATCH 1/2] dynamic_debug: drop obsolete trim_prefix

Some time ago, __FILE__ on gcc kernel builds changed from a full path to
a source-root-relative path.  Presumably this happened after

2b6783191da7 dynamic_debug: add trim_prefix() to provide source-root relative paths

That patch added trim_prefix() to improve readability by shortening
lines in control file, in vpr_info output, and to accept relative
filenames in ddebug_query(), improving ease of use.

At any rate, __FILE__ now yields source-root-relative names, so
trim_prefix() is worse than obsolete; it has code to skip common
prefixes where used (see above), which can hide bugs in adaptations
for 2 kinds of uses. Moreover, that code uses __FILE__ and was thus
unnaffected by the behavior change.  Now it just obfuscates.

So remove fn, and 3 uses.  The query case deserves a closer look.
Per Documentation/admin-guide/dynamic-debug-howto.rst:

file
    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

The 3rd use was the original form, and the least useful (except for
copy-paste of full-paths).  It is not properly supported by current
code, as __FILE__ has already stripped ~/projects/linux.git/,
defeating the prefix-skip code on a "filename <full-path>" search.

Since it already broken, lets just remove it.  wildcard match added
since should help ease transition.
---
 lib/dynamic_debug.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c60409138e13..ba35199edd9c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -67,17 +67,6 @@ static LIST_HEAD(ddebug_tables);
 static int verbose;
 module_param(verbose, int, 0644);
 
-/* 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' },
@@ -162,9 +151,7 @@ static int ddebug_change(const struct ddebug_query *query,
 			if (query->filename &&
 			    !match_wildcard(query->filename, dp->filename) &&
 			    !match_wildcard(query->filename,
-					   kbasename(dp->filename)) &&
-			    !match_wildcard(query->filename,
-					   trim_prefix(dp->filename)))
+					    kbasename(dp->filename)))
 				continue;
 
 			/* match against the function */
@@ -199,7 +186,7 @@ static int ddebug_change(const struct ddebug_query *query,
 #endif
 			dp->flags = newflags;
 			vpr_info("changed %s:%d [%s]%s =%s\n",
-				 trim_prefix(dp->filename), dp->lineno,
+				 dp->filename, dp->lineno,
 				 dt->mod_name, dp->function,
 				 ddebug_describe_flags(dp, flagbuf,
 						       sizeof(flagbuf)));
@@ -827,7 +814,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	}
 
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
-		   trim_prefix(dp->filename), dp->lineno,
+		   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\"");
-- 
2.21.0


[-- Attachment #3: 0002-dynamic_debug-drop-overwrought-comment.patch --]
[-- Type: text/x-patch, Size: 1099 bytes --]

From bc7b9ec150ce0f74140c5f4fe222b6971b22f5cf Mon Sep 17 00:00:00 2001
From: Jim Cromie <jim.cromie@gmail.com>
Date: Thu, 5 Sep 2019 02:11:01 -0600
Subject: [PATCH 2/2] dynamic_debug: drop overwrought comment

4bad78c55002 lib/dynamic_debug.c: use seq_open_private() instead of seq_open()

cleaned up ddebug_proc_open, obsoleting the comment about that code.
---
 lib/dynamic_debug.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ba35199edd9c..8fb140a55ad3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -840,13 +840,6 @@ static const struct seq_operations ddebug_proc_seqops = {
 	.stop = ddebug_proc_stop
 };
 
-/*
- * 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)
 {
 	vpr_info("called\n");
-- 
2.21.0


[-- Attachment #4: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

  reply	other threads:[~2019-09-05 14:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-04 21:42 is there a macro symbol for strlen($LINUX_SOURCE_PATH) jim.cromie
2019-09-04 22:18 ` Varun Iyer
2019-09-05 14:31   ` jim.cromie [this message]
2019-09-05 20:23     ` Valdis Klētnieks
2019-09-05  5:47 ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJfuBxwChECpA8j+GWdyy+wAtydFybjDLSCQjcf7C-L0Oa2iQA@mail.gmail.com \
    --to=jim.cromie@gmail.com \
    --cc=greg@kroah.com \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=varun_iyer@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).