On Tue, Apr 27, 2021 at 11:41 PM Christoph Hellwig <hch@xxxxxx> wrote:
>
> "you guys" here is purely me, so I take the blame. And no, I actually
> did have a first version usind %pD, tested it and looked at the output
> and saw how it stripped the actual useful part of the path, that is the
> first components.
So that's why I cc'd Al and Jia.
You may not have realized that the default for %pD is to show only one
component, and if you want to see more, you need to use something like
%pD4.
Which should be _plenty_.
But it's also something where I think that default (ie "no number")
behavior may be a bit surprising, and perhaps not the greatest
interface.
So let me just quote that first reply of mine, because you seem to not
have seen it:
> We have '%pD' for printing a filename. It may not be perfect (by
> default it only prints one component, you can do "%pD4" to show up to
> four components), but it should "JustWork(tm)".
>
> And if it doesn't, we should fix it.
I really think %pD4 should be more than good enough. And I think maybe
we should make plain "%pD" mean "as much of the path that is
reasonable" rather than "as few components as possible" (ie 1).
So I don't think "%pD" (or "%pD4") is necessarily perfect, but I think
it's even worse when people then go and do odd ad-hoc things because
of some inconvenience in our %pD implementation.
For example, changing the default to be "show more by default" should
be as simple as something like the attached. I do think that would be
the more natural behavior for %pD - don't limit it unnecessarily by
default, but for somebody who literally just wants to see a maximum of
2 components, using '%pD2' makes sense.
(Similarly, changing the limit of 4 components to something slightly
bigger would be trivial)
Hmm?
Grepping for existing users with
git grep '%pD[^1-4]'
most of them would probably like a full pathname, and the odd s390
hmcdrv_dev.c use should just be fixed (it has a hardcoded "/dev/%pD",
which seems very wrong).
Of course, %pD has some other limitations too. It doesn't follow
mount-points up. It's kind of intentionally a "for simple
informational uses only", but good enough in practice exactly for
things like debug printouts.
Linus
lib/vsprintf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6c56c62fd9a5..5b563953f970 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -880,11 +880,11 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
int i, n;
switch (fmt[1]) {
- case '2': case '3': case '4':
+ case '1': case '2': case '3': case '4':
depth = fmt[1] - '0';
break;
default:
- depth = 1;
+ depth = 4;
}
rcu_read_lock();
