Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
* is there a macro symbol for strlen($LINUX_SOURCE_PATH)
@ 2019-09-04 21:42 jim.cromie
  2019-09-04 22:18 ` Varun Iyer
  2019-09-05  5:47 ` Greg KH
  0 siblings, 2 replies; 5+ messages in thread
From: jim.cromie @ 2019-09-04 21:42 UTC (permalink / raw)
  To: kernelnewbies

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.

Id hazard a guess that something like __RELPATH_FILE__
would work in many cases and perhaps even be more readable
(by reducing long strings) in the generated output

Do the macros exist to cobble this together for compile-time ?
Does it warrant a short name ? either to wrap __FILE__ uses or replace them ?

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

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

* Re: is there a macro symbol for strlen($LINUX_SOURCE_PATH)
  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
  2019-09-05  5:47 ` Greg KH
  1 sibling, 1 reply; 5+ messages in thread
From: Varun Iyer @ 2019-09-04 22:18 UTC (permalink / raw)
  To: jim.cromie; +Cc: kernelnewbies

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.

> 
> Id hazard a guess that something like __RELPATH_FILE__
> would work in many cases and perhaps even be more readable
> (by reducing long strings) in the generated output
> 
> Do the macros exist to cobble this together for compile-time ?
> Does it warrant a short name ? either to wrap __FILE__ uses or replace them ?
> 
> _______________________________________________
> Kernelnewbies mailing list
> Kernelnewbies@kernelnewbies.org
> https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

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

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

* Re: is there a macro symbol for strlen($LINUX_SOURCE_PATH)
  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  5:47 ` Greg KH
  1 sibling, 0 replies; 5+ messages in thread
From: Greg KH @ 2019-09-05  5:47 UTC (permalink / raw)
  To: jim.cromie; +Cc: kernelnewbies

On Wed, Sep 04, 2019 at 03:42:01PM -0600, jim.cromie@gmail.com wrote:
> in the kernel, there are a lot of usages of __FILE__

And almost all of them should go away, I think checkpatch.pl will
mention something like that.  I know I will gladly take patches that fix
up printk messages that have __FILE__ in them to use the correct macros
instead for portions of the kernel that I maintain.

thanks,

greg k-h

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

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

* Re: is there a macro symbol for strlen($LINUX_SOURCE_PATH)
  2019-09-04 22:18 ` Varun Iyer
@ 2019-09-05 14:31   ` jim.cromie
  2019-09-05 20:23     ` Valdis Klētnieks
  0 siblings, 1 reply; 5+ messages in thread
From: jim.cromie @ 2019-09-05 14:31 UTC (permalink / raw)
  To: Varun Iyer; +Cc: Greg KH, kernelnewbies

[-- 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

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

* Re: is there a macro symbol for strlen($LINUX_SOURCE_PATH)
  2019-09-05 14:31   ` jim.cromie
@ 2019-09-05 20:23     ` Valdis Klētnieks
  0 siblings, 0 replies; 5+ messages in thread
From: Valdis Klētnieks @ 2019-09-05 20:23 UTC (permalink / raw)
  To: jim.cromie; +Cc: Greg KH, Varun Iyer, kernelnewbies

[-- Attachment #1.1: Type: text/plain, Size: 918 bytes --]

On Thu, 05 Sep 2019 08:31:55 -0600, jim.cromie@gmail.com said:

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

You should find a way to test that this is TRTTD for all gcc releases still
supported for building a kernel (which may mean finding a 4.8 or 4.9 to
test on to see if it uses relative or full paths).

Removing these functions for kernels built with pre-change gcc will cause some
semantic changes. Probably the *right* thing to do is to figure out what
release it was changed in, and do some hacking to include/config/compiler-gcc.h.

In addition, any such patches should be at least non-hostile to the ongoing
effort to get a kernel tree that builds with clang rather than gcc.


[-- Attachment #1.2: Type: application/pgp-signature, Size: 832 bytes --]

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

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

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2019-09-05 20:23     ` Valdis Klētnieks
2019-09-05  5:47 ` Greg KH

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/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 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org kernelnewbies@archiver.kernel.org
	public-inbox-index kernelnewbies


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


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