Background
==========
Linus suggested printing the full path of file instead of printing
the components as '%pd'.
Typically, there is no need for printk specifiers to take any real locks
(ie mount_lock or rename_lock). So I introduce a new helper d_path_fast
which is similar to d_path except it doesn't take any seqlock/spinlock.
This series is based on Al Viro's d_path cleanup patches [1] which
lifted the inner lockless loop into a new helper.
Link: https://lkml.org/lkml/2021/5/18/1260 [1]
Test
====
The cases I tested:
1. print '%pD' with full path of ext4 file
2. mount a ext4 filesystem upon a ext4 filesystem, and print the file
with '%pD'
3. all test_print selftests, including the new '%14pD' '%-14pD'
4. kasnprintf
Changelog
=========
v5:
- remove the RFC tag
- refine the commit msg/comments(by Petr, Andy)
- make using_scratch_space a new parameter of the test case
v4:
- don't support spec.precision anymore for '%pD'
- add Rasmus's patch into this series
v3:
- implement new d_path_unsafe to use [buf, end] instead of stack space for
filling bytes (by Matthew)
- add new test cases for '%pD'
- drop patch "hmcdrv: remove the redundant directory path" before removing rfc.
v2:
- implement new d_path_fast based on Al Viro's patches
- add check_pointer check (by Petr)
- change the max full path size to 256 in stack space
v1: https://lkml.org/lkml/2021/5/8/122
Jia He (3):
fs: introduce helper d_path_unsafe()
lib/vsprintf.c: make '%pD' print the full path of file
lib/test_printf.c: add test cases for '%pD'
Rasmus Villemoes (1):
lib/test_printf.c: split write-beyond-buffer check in two
Documentation/core-api/printk-formats.rst | 5 +-
fs/d_path.c | 104 +++++++++++++++++++++-
include/linux/dcache.h | 1 +
lib/test_printf.c | 54 ++++++++---
lib/vsprintf.c | 40 ++++++++-
5 files changed, 184 insertions(+), 20 deletions(-)
--
2.17.1
This helper is similar to d_path() except that it doesn't take any
seqlock/spinlock. It is typical for debugging purposes. Besides,
an additional return value *prenpend_len* is used to get the full
path length of the dentry, ingoring the tail '\0'.
the full path length = end - buf - prepend_length - 1
Previously it will skip the prepend_name() loop at once in
__prepen_path() when the buffer length is not enough or even negative.
prepend_name_with_len() will get the full length of dentry name
together with the parent recursively regardless of the buffer length.
If someone invokes snprintf() with small but positive space,
prepend_name_with_len() moves and copies the string partially.
More than that, kasprintf() will pass NULL _buf_ and _end_ as the
parameters. Hence return at the very beginning with false in this case.
Suggested-by: Matthew Wilcox <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
fs/d_path.c | 104 +++++++++++++++++++++++++++++++++++++++--
include/linux/dcache.h | 1 +
2 files changed, 101 insertions(+), 4 deletions(-)
diff --git a/fs/d_path.c b/fs/d_path.c
index 23a53f7b5c71..84a738375698 100644
--- a/fs/d_path.c
+++ b/fs/d_path.c
@@ -33,8 +33,7 @@ static void prepend(struct prepend_buffer *p, const char *str, int namelen)
/**
* prepend_name - prepend a pathname in front of current buffer pointer
- * @buffer: buffer pointer
- * @buflen: allocated length of the buffer
+ * @p: prepend buffer which contains buffer pointer and allocated length
* @name: name string and length qstr structure
*
* With RCU path tracing, it may race with d_move(). Use READ_ONCE() to
@@ -68,9 +67,84 @@ static bool prepend_name(struct prepend_buffer *p, const struct qstr *name)
return true;
}
+/**
+ * prepend_name_with_len - prepend a pathname in front of current buffer
+ * pointer with limited orig_buflen.
+ * @p: prepend buffer which contains buffer pointer and allocated length
+ * @name: name string and length qstr structure
+ * @orig_buflen original length of the buffer.
+ *
+ * With the original length of the buffer (p.ptr is changable), the dentry
+ * name string will be filled into the prepending buffer. Given the orginal
+ * length might be less than name string, the dentry name can be moved or
+ * truncated.
+ *
+ * Load acquire is needed to make sure that we see that terminating NUL,
+ * which is similar to prepend_name().
+ */
+static bool prepend_name_with_len(struct prepend_buffer *p,
+ const struct qstr *name, int orig_buflen)
+{
+ const char *dname = smp_load_acquire(&name->name); /* ^^^ */
+ int dlen = READ_ONCE(name->len);
+ char *s;
+ int last_len = p->len;
+
+ p->len -= dlen + 1;
+
+ if (unlikely(!p->buf))
+ return false;
+
+ if (orig_buflen <= 0)
+ return false;
+
+ /*
+ * The first time we overflow the buffer. Then fill the string
+ * partially from the beginning
+ */
+ if (unlikely(p->len < 0)) {
+ int buflen = strlen(p->buf);
+
+ /* memcpy src */
+ s = p->buf;
+
+ /* Still have small space to fill partially */
+ if (last_len > 0) {
+ p->buf -= last_len;
+ buflen += last_len;
+ }
+
+ if (buflen > dlen + 1) {
+ /* Dentry name can be fully filled */
+ memmove(p->buf + dlen + 1, s, buflen - dlen - 1);
+ p->buf[0] = '/';
+ memcpy(p->buf + 1, dname, dlen);
+ } else if (buflen > 0) {
+ /* Can be partially filled, and drop last dentry */
+ p->buf[0] = '/';
+ memcpy(p->buf + 1, dname, buflen - 1);
+ }
+
+ return false;
+ }
+
+ s = p->buf -= dlen + 1;
+ *s++ = '/';
+ while (dlen--) {
+ char c = *dname++;
+
+ if (!c)
+ break;
+ *s++ = c;
+ }
+ return true;
+}
+
static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
const struct path *root, struct prepend_buffer *p)
{
+ int orig_buflen = p->len;
+
while (dentry != root->dentry || &mnt->mnt != root->mnt) {
const struct dentry *parent = READ_ONCE(dentry->d_parent);
@@ -97,8 +171,7 @@ static int __prepend_path(const struct dentry *dentry, const struct mount *mnt,
return 3;
prefetch(parent);
- if (!prepend_name(p, &dentry->d_name))
- break;
+ prepend_name_with_len(p, &dentry->d_name, orig_buflen);
dentry = parent;
}
return 0;
@@ -263,6 +336,29 @@ char *d_path(const struct path *path, char *buf, int buflen)
}
EXPORT_SYMBOL(d_path);
+/**
+ * d_path_unsafe - return the full path of a dentry without taking
+ * any seqlock/spinlock. This helper is typical for debugging purposes.
+ */
+char *d_path_unsafe(const struct path *path, char *buf, int buflen,
+ int *prepend_len)
+{
+ struct path root;
+ struct mount *mnt = real_mount(path->mnt);
+ DECLARE_BUFFER(b, buf, buflen);
+
+ rcu_read_lock();
+ get_fs_root_rcu(current->fs, &root);
+
+ prepend(&b, "", 1);
+ __prepend_path(path->dentry, mnt, &root, &b);
+ rcu_read_unlock();
+
+ *prepend_len = b.len;
+
+ return b.buf;
+}
+
/*
* Helper function for dentry_operations.d_dname() members
*/
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 9e23d33bb6f1..ec118b684055 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -301,6 +301,7 @@ char *dynamic_dname(struct dentry *, char *, int, const char *, ...);
extern char *__d_path(const struct path *, const struct path *, char *, int);
extern char *d_absolute_path(const struct path *, char *, int);
extern char *d_path(const struct path *, char *, int);
+extern char *d_path_unsafe(const struct path *, char *, int, int*);
extern char *dentry_path_raw(const struct dentry *, char *, int);
extern char *dentry_path(const struct dentry *, char *, int);
--
2.17.1
Previously, the specifier '%pD' is for printing dentry name of struct
file. It may not be perfect (by default it only prints one component.)
As suggested by Linus [1]:
A dentry has a parent, but at the same time, a dentry really does
inherently have "one name" (and given just the dentry pointers, you
can't show mount-related parenthood, so in many ways the "show just
one name" makes sense for "%pd" in ways it doesn't necessarily for
"%pD"). But while a dentry arguably has that "one primary component",
a _file_ is certainly not exclusively about that last component.
Hence change the behavior of '%pD' to print the full path of that file.
Precision is never going to be used with %p (or any of its kernel
extensions) if -Wformat is turned on.
Link: https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-ru+iktb4MYbMQG1rnZ81dXYFVg@mail.gmail.com/ [1]
Suggested-by: Linus Torvalds <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
Documentation/core-api/printk-formats.rst | 5 +--
lib/vsprintf.c | 40 ++++++++++++++++++++---
2 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst
index f063a384c7c8..95ba14dc529b 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -408,12 +408,13 @@ dentry names
::
%pd{,2,3,4}
- %pD{,2,3,4}
+ %pD
For printing dentry name; if we race with :c:func:`d_move`, the name might
be a mix of old and new ones, but it won't oops. %pd dentry is a safer
equivalent of %s dentry->d_name.name we used to use, %pd<n> prints ``n``
-last components. %pD does the same thing for struct file.
+last components. %pD prints full file path together with mount-related
+parenthood.
Passed by reference.
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index f0c35d9b65bf..9944b70311de 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -26,6 +26,7 @@
#include <linux/types.h>
#include <linux/string.h>
#include <linux/ctype.h>
+#include <linux/dcache.h>
#include <linux/kernel.h>
#include <linux/kallsyms.h>
#include <linux/math64.h>
@@ -920,13 +921,44 @@ char *dentry_name(char *buf, char *end, const struct dentry *d, struct printf_sp
}
static noinline_for_stack
-char *file_dentry_name(char *buf, char *end, const struct file *f,
+char *file_d_path_name(char *buf, char *end, const struct file *f,
struct printf_spec spec, const char *fmt)
{
+ const struct path *path;
+ char *p;
+ int prepend_len, widen_len, dpath_len;
+
if (check_pointer(&buf, end, f, spec))
return buf;
- return dentry_name(buf, end, f->f_path.dentry, spec, fmt);
+ path = &f->f_path;
+ if (check_pointer(&buf, end, path, spec))
+ return buf;
+
+ p = d_path_unsafe(path, buf, end - buf, &prepend_len);
+
+ /* Calculate the full d_path length, ignoring the tail '\0' */
+ dpath_len = end - buf - prepend_len - 1;
+
+ widen_len = max_t(int, dpath_len, spec.field_width);
+
+ /* Case 1: Already started past the buffer. Just forward @buf. */
+ if (buf >= end)
+ return buf + widen_len;
+
+ /*
+ * Case 2: The entire remaining space of the buffer filled by
+ * the truncated path. Still need to get moved right when
+ * the filled width is greather than the full path length.
+ */
+ if (prepend_len < 0)
+ return widen_string(buf + dpath_len, dpath_len, end, spec);
+
+ /*
+ * Case 3: The full path is printed at the end of the buffer.
+ * Print it at the right location in the same buffer.
+ */
+ return string_nocheck(buf, end, p, spec);
}
#ifdef CONFIG_BLOCK
static noinline_for_stack
@@ -2296,7 +2328,7 @@ early_param("no_hash_pointers", no_hash_pointers_enable);
* - 'a[pd]' For address types [p] phys_addr_t, [d] dma_addr_t and derivatives
* (default assumed to be phys_addr_t, passed by reference)
* - 'd[234]' For a dentry name (optionally 2-4 last components)
- * - 'D[234]' Same as 'd' but for a struct file
+ * - 'D' For full path name of a struct file
* - 'g' For block_device name (gendisk + partition number)
* - 't[RT][dt][r]' For time and date as represented by:
* R struct rtc_time
@@ -2395,7 +2427,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
case 'C':
return clock(buf, end, ptr, spec, fmt);
case 'D':
- return file_dentry_name(buf, end, ptr, spec, fmt);
+ return file_d_path_name(buf, end, ptr, spec, fmt);
#ifdef CONFIG_BLOCK
case 'g':
return bdev_name(buf, end, ptr, spec, fmt);
--
2.17.1
From: Rasmus Villemoes <[email protected]>
Before each invocation of vsnprintf(), do_test() memsets the entire
allocated buffer to a sentinel value. That buffer includes leading and
trailing padding which is never included in the buffer area handed to
vsnprintf (spaces merely for clarity):
pad test_buffer pad
**** **************** ****
Then vsnprintf() is invoked with a bufsize argument <=
BUF_SIZE. Suppose bufsize=10, then we'd have e.g.
|pad | test_buffer |pad |
**** pizza0 **** ****** ****
A B C D E
where vsnprintf() was given the area from B to D.
It is obviously a bug for vsnprintf to touch anything between A and B
or between D and E. The former is checked for as one would expect. But
for the latter, we are actually a little stricter in that we check the
area between C and E.
Split that check in two, providing a clearer error message in case it
was a genuine buffer overrun and not merely a write within the
provided buffer, but after the end of the generated string.
So far, no part of the vsnprintf() implementation has had any use for
using the whole buffer as scratch space, but it's not unreasonable to
allow that, as long as the result is properly nul-terminated and the
return value is the right one. However, it is somewhat unusual, and
most %<something> won't need this, so keep the [C,D] check, but make
it easy for a later patch to make that part opt-out for certain tests.
Signed-off-by: Rasmus Villemoes <[email protected]>
Tested-by: Jia He <[email protected]>
Signed-off-by: Jia He <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
---
lib/test_printf.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index ec0d5976bb69..d1d2f898ebae 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -78,12 +78,17 @@ do_test(int bufsize, const char *expect, int elen,
return 1;
}
- if (memchr_inv(test_buffer + written + 1, FILL_CHAR, BUF_SIZE + PAD_SIZE - (written + 1))) {
+ if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
bufsize, fmt);
return 1;
}
+ if (memchr_inv(test_buffer + bufsize, FILL_CHAR, BUF_SIZE + PAD_SIZE - bufsize)) {
+ pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond buffer\n", bufsize, fmt);
+ return 1;
+ }
+
if (memcmp(test_buffer, expect, written)) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote '%s', expected '%.*s'\n",
bufsize, fmt, test_buffer, written, expect);
--
2.17.1
After the behaviour of specifier '%pD' is changed to print the full path
of struct file, the related test cases are also updated.
Given the full path string of '%pD' is prepended from the end of the scratch
buffer, the check of "wrote beyond the nul-terminator" should be skipped
for '%pD'.
Parameterize the new using_scratch_space in __test, do_test to skip the
test case mentioned above,
Signed-off-by: Jia He <[email protected]>
---
lib/test_printf.c | 49 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 10 deletions(-)
diff --git a/lib/test_printf.c b/lib/test_printf.c
index d1d2f898ebae..f48da88bc77b 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -16,6 +16,7 @@
#include <linux/bitmap.h>
#include <linux/dcache.h>
+#include <linux/fs.h>
#include <linux/socket.h>
#include <linux/in.h>
@@ -37,8 +38,8 @@ static char *alloced_buffer __initdata;
extern bool no_hash_pointers;
-static int __printf(4, 0) __init
-do_test(int bufsize, const char *expect, int elen,
+static int __printf(5, 0) __init
+do_test(int bufsize, const char *expect, int elen, bool using_scratch_space,
const char *fmt, va_list ap)
{
va_list aq;
@@ -78,7 +79,7 @@ do_test(int bufsize, const char *expect, int elen,
return 1;
}
- if (memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
+ if (!using_scratch_space && memchr_inv(test_buffer + written + 1, FILL_CHAR, bufsize - (written + 1))) {
pr_warn("vsnprintf(buf, %d, \"%s\", ...) wrote beyond the nul-terminator\n",
bufsize, fmt);
return 1;
@@ -97,8 +98,9 @@ do_test(int bufsize, const char *expect, int elen,
return 0;
}
-static void __printf(3, 4) __init
-__test(const char *expect, int elen, const char *fmt, ...)
+static void __printf(4, 5) __init
+__test(const char *expect, int elen, bool using_scratch_space,
+ const char *fmt, ...)
{
va_list ap;
int rand;
@@ -119,11 +121,11 @@ __test(const char *expect, int elen, const char *fmt, ...)
* enough and 0), and then we also test that kvasprintf would
* be able to print it as expected.
*/
- failed_tests += do_test(BUF_SIZE, expect, elen, fmt, ap);
+ failed_tests += do_test(BUF_SIZE, expect, elen, using_scratch_space, fmt, ap);
rand = 1 + prandom_u32_max(elen+1);
/* Since elen < BUF_SIZE, we have 1 <= rand <= BUF_SIZE. */
- failed_tests += do_test(rand, expect, elen, fmt, ap);
- failed_tests += do_test(0, expect, elen, fmt, ap);
+ failed_tests += do_test(rand, expect, elen, using_scratch_space, fmt, ap);
+ failed_tests += do_test(0, expect, elen, using_scratch_space, fmt, ap);
p = kvasprintf(GFP_KERNEL, fmt, ap);
if (p) {
@@ -138,8 +140,15 @@ __test(const char *expect, int elen, const char *fmt, ...)
va_end(ap);
}
+/*
+ * More relaxed test for non-standard formats that are using the provided buffer
+ * as a scratch space and write beyond the trailing '\0'.
+ */
+#define test_using_scratch_space(expect, fmt, ...) \
+ __test(expect, strlen(expect), true, fmt, ##__VA_ARGS__)
+
#define test(expect, fmt, ...) \
- __test(expect, strlen(expect), fmt, ##__VA_ARGS__)
+ __test(expect, strlen(expect), false, fmt, ##__VA_ARGS__)
static void __init
test_basic(void)
@@ -150,7 +159,7 @@ test_basic(void)
test("", &nul);
test("100%", "100%%");
test("xxx%yyy", "xxx%cyyy", '%');
- __test("xxx\0yyy", 7, "xxx%cyyy", '\0');
+ __test("xxx\0yyy", 7, false, "xxx%cyyy", '\0');
}
static void __init
@@ -501,6 +510,25 @@ dentry(void)
test(" bravo/alfa| bravo/alfa", "%12pd2|%*pd2", &test_dentry[2], 12, &test_dentry[2]);
}
+static struct vfsmount test_vfsmnt __initdata = {};
+
+static struct file test_file __initdata = {
+ .f_path = { .dentry = &test_dentry[2],
+ .mnt = &test_vfsmnt,
+ },
+};
+
+static void __init
+f_d_path(void)
+{
+ test("(null)", "%pD", NULL);
+ test("(efault)", "%pD", PTR_INVALID);
+
+ test_using_scratch_space("/bravo/alfa |/bravo/alfa ", "%-14pD|%*pD", &test_file, -14, &test_file);
+ test_using_scratch_space(" /bravo/alfa| /bravo/alfa", "%14pD|%*pD", &test_file, 14, &test_file);
+ test_using_scratch_space(" /bravo/alfa|/bravo/alfa ", "%14pD|%-14pD", &test_file, &test_file);
+}
+
static void __init
struct_va_format(void)
{
@@ -784,6 +812,7 @@ test_pointer(void)
ip();
uuid();
dentry();
+ f_d_path();
struct_va_format();
time_and_date();
struct_clk();
--
2.17.1
On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote:
> This helper is similar to d_path() except that it doesn't take any
> seqlock/spinlock. It is typical for debugging purposes. Besides,
> an additional return value *prenpend_len* is used to get the full
> path length of the dentry, ingoring the tail '\0'.
> the full path length = end - buf - prepend_length - 1
Missed period at the end of sentence.
> Previously it will skip the prepend_name() loop at once in
> __prepen_path() when the buffer length is not enough or even negative.
> prepend_name_with_len() will get the full length of dentry name
> together with the parent recursively regardless of the buffer length.
> If someone invokes snprintf() with small but positive space,
> prepend_name_with_len() moves and copies the string partially.
>
> More than that, kasprintf() will pass NULL _buf_ and _end_ as the
> parameters. Hence return at the very beginning with false in this case.
These two paragraphs are talking about printf() interface, while patch has
nothing to do with it. Please, rephrase in a way that it doesn't refer to the
particular callers. Better to mention them in the corresponding printf()
patch(es).
...
> * prepend_name - prepend a pathname in front of current buffer pointer
> - * @buffer: buffer pointer
> - * @buflen: allocated length of the buffer
> + * @p: prepend buffer which contains buffer pointer and allocated length
> * @name: name string and length qstr structure
Indentation issue btw, can be fixed in the same patch.
> *
> * With RCU path tracing, it may race with d_move(). Use READ_ONCE() to
Shouldn't this be a separate change with corresponding Fixes tag?
...
> +/**
> + * d_path_unsafe - return the full path of a dentry without taking
> + * any seqlock/spinlock. This helper is typical for debugging purposes.
Seems you ignored my comment, or forget to test, or compile test with kernel
doc validator enabled doesn't show any issues. If it's the latter, we have to
fix kernel doc validator.
TL;DR: describe parameters as well.
> + */
...
> + struct path root;
> + struct mount *mnt = real_mount(path->mnt);
> + DECLARE_BUFFER(b, buf, buflen);
Can wee keep this in reversed xmas tree order?
--
With Best Regards,
Andy Shevchenko
On Tue, Jun 22, 2021 at 10:06:32PM +0800, Jia He wrote:
> Previously, the specifier '%pD' is for printing dentry name of struct
> file. It may not be perfect (by default it only prints one component.)
>
> As suggested by Linus [1]:
Citing is better looked when you shift right it by two white spaces.
> A dentry has a parent, but at the same time, a dentry really does
> inherently have "one name" (and given just the dentry pointers, you
> can't show mount-related parenthood, so in many ways the "show just
> one name" makes sense for "%pd" in ways it doesn't necessarily for
> "%pD"). But while a dentry arguably has that "one primary component",
> a _file_ is certainly not exclusively about that last component.
>
> Hence change the behavior of '%pD' to print the full path of that file.
>
> Precision is never going to be used with %p (or any of its kernel
> extensions) if -Wformat is turned on.
> Link: https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-ru+iktb4MYbMQG1rnZ81dXYFVg@mail.gmail.com/ [1]
>
Shouldn't be blank lines in the tag block. I have an impression that I have
commented on this already...
...
> -last components. %pD does the same thing for struct file.
> +last components. %pD prints full file path together with mount-related
I guess you may also convert double space to a single one.
> +parenthood.
--
With Best Regards,
Andy Shevchenko
On Tue, Jun 22, 2021 at 10:06:34PM +0800, Jia He wrote:
> After the behaviour of specifier '%pD' is changed to print the full path
> of struct file, the related test cases are also updated.
>
> Given the full path string of '%pD' is prepended from the end of the scratch
> buffer, the check of "wrote beyond the nul-terminator" should be skipped
> for '%pD'.
>
> Parameterize the new using_scratch_space in __test, do_test to skip the
__test()
> test case mentioned above,
--
With Best Regards,
Andy Shevchenko
On Tue, Jun 22, 2021 at 10:06:30PM +0800, Jia He wrote:
> Background
> ==========
> Linus suggested printing the full path of file instead of printing
> the components as '%pd'.
>
> Typically, there is no need for printk specifiers to take any real locks
> (ie mount_lock or rename_lock). So I introduce a new helper d_path_fast
> which is similar to d_path except it doesn't take any seqlock/spinlock.
>
> This series is based on Al Viro's d_path cleanup patches [1] which
> lifted the inner lockless loop into a new helper.
>
> Link: https://lkml.org/lkml/2021/5/18/1260 [1]
>
> Test
> ====
> The cases I tested:
> 1. print '%pD' with full path of ext4 file
> 2. mount a ext4 filesystem upon a ext4 filesystem, and print the file
> with '%pD'
> 3. all test_print selftests, including the new '%14pD' '%-14pD'
> 4. kasnprintf
I believe you are talking about kasprintf().
> Changelog
> =========
> v5:
> - remove the RFC tag
JFYI, when we drop RFC we usually start the series from v1.
> - refine the commit msg/comments(by Petr, Andy)
> - make using_scratch_space a new parameter of the test case
Thanks for the update, I have found few minor things, please address them and
feel free to add
Reviewed-by: Andy Shevchenko <[email protected]>
> v4:
> - don't support spec.precision anymore for '%pD'
> - add Rasmus's patch into this series
>
> v3:
> - implement new d_path_unsafe to use [buf, end] instead of stack space for
> filling bytes (by Matthew)
> - add new test cases for '%pD'
> - drop patch "hmcdrv: remove the redundant directory path" before removing rfc.
>
> v2:
> - implement new d_path_fast based on Al Viro's patches
> - add check_pointer check (by Petr)
> - change the max full path size to 256 in stack space
>
> v1: https://lkml.org/lkml/2021/5/8/122
>
>
> Jia He (3):
> fs: introduce helper d_path_unsafe()
> lib/vsprintf.c: make '%pD' print the full path of file
> lib/test_printf.c: add test cases for '%pD'
>
> Rasmus Villemoes (1):
> lib/test_printf.c: split write-beyond-buffer check in two
>
> Documentation/core-api/printk-formats.rst | 5 +-
> fs/d_path.c | 104 +++++++++++++++++++++-
> include/linux/dcache.h | 1 +
> lib/test_printf.c | 54 ++++++++---
> lib/vsprintf.c | 40 ++++++++-
> 5 files changed, 184 insertions(+), 20 deletions(-)
>
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
On 22/06/2021 16.06, Jia He wrote:
> After the behaviour of specifier '%pD' is changed to print the full path
> of struct file, the related test cases are also updated.
>
> Given the full path string of '%pD' is prepended from the end of the scratch
> buffer, the check of "wrote beyond the nul-terminator" should be skipped
> for '%pD'.
>
> Parameterize the new using_scratch_space in __test, do_test to skip the
> test case mentioned above,
I actually prefer the first suggestion of just having a file-global bool.
If and when we get other checks that need to be done selectively [e.g.
"snprintf into a too short buffer produces a prefix of the full string",
which also came up during this discussion but was ultimately kept]
depending on the %<whatever> being exercised, we can add a "u32 nocheck"
with a bunch of bits saying what to elide.
Not insisting either way, just my $0.02.
Rasmus
Hi Andy
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Tuesday, June 22, 2021 10:37 PM
> To: Justin He <[email protected]>
> Cc: Petr Mladek <[email protected]>; Steven Rostedt <[email protected]>;
> Sergey Senozhatsky <[email protected]>; Rasmus Villemoes
> <[email protected]>; Jonathan Corbet <[email protected]>; Alexander
> Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Zijlstra (Intel) <[email protected]>; Eric
> Biggers <[email protected]>; Ahmed S. Darwish <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; Matthew Wilcox <[email protected]>; Christoph
> Hellwig <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH v5 1/4] fs: introduce helper d_path_unsafe()
>
> On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote:
> > This helper is similar to d_path() except that it doesn't take any
> > seqlock/spinlock. It is typical for debugging purposes. Besides,
> > an additional return value *prenpend_len* is used to get the full
> > path length of the dentry, ingoring the tail '\0'.
> > the full path length = end - buf - prepend_length - 1
>
> Missed period at the end of sentence.
>
Okay
> > Previously it will skip the prepend_name() loop at once in
> > __prepen_path() when the buffer length is not enough or even negative.
> > prepend_name_with_len() will get the full length of dentry name
> > together with the parent recursively regardless of the buffer length.
>
> > If someone invokes snprintf() with small but positive space,
> > prepend_name_with_len() moves and copies the string partially.
> >
> > More than that, kasprintf() will pass NULL _buf_ and _end_ as the
> > parameters. Hence return at the very beginning with false in this case.
>
> These two paragraphs are talking about printf() interface, while patch has
> nothing to do with it. Please, rephrase in a way that it doesn't refer to
> the
> particular callers. Better to mention them in the corresponding printf()
> patch(es).
>
Okay
> ...
>
> > * prepend_name - prepend a pathname in front of current buffer pointer
> > - * @buffer: buffer pointer
> > - * @buflen: allocated length of the buffer
> > + * @p: prepend buffer which contains buffer pointer and allocated length
>
> > * @name: name string and length qstr structure
>
> Indentation issue btw, can be fixed in the same patch.
Okay
>
> > *
> > * With RCU path tracing, it may race with d_move(). Use READ_ONCE() to
>
> Shouldn't this be a separate change with corresponding Fixes tag?
Sorry, I don't quite understand here.
What do you want to fix?
>
> ...
>
> > +/**
> > + * d_path_unsafe - return the full path of a dentry without taking
> > + * any seqlock/spinlock. This helper is typical for debugging purposes.
>
> Seems you ignored my comment, or forget to test, or compile test with
> kernel
> doc validator enabled doesn't show any issues. If it's the latter, we have
> to
> fix kernel doc validator.
>
> TL;DR: describe parameters as well.
>
My bad. Apologize for that.
> > + */
>
> ...
>
> > + struct path root;
> > + struct mount *mnt = real_mount(path->mnt);
> > + DECLARE_BUFFER(b, buf, buflen);
>
> Can wee keep this in reversed xmas tree order?
Sure ????
--
Cheers,
Justin (Jia He)
Hi Andy
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Tuesday, June 22, 2021 10:40 PM
> To: Justin He <[email protected]>
> Cc: Petr Mladek <[email protected]>; Steven Rostedt <[email protected]>;
> Sergey Senozhatsky <[email protected]>; Rasmus Villemoes
> <[email protected]>; Jonathan Corbet <[email protected]>; Alexander
> Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Zijlstra (Intel) <[email protected]>; Eric
> Biggers <[email protected]>; Ahmed S. Darwish <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; Matthew Wilcox <[email protected]>; Christoph
> Hellwig <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH v5 2/4] lib/vsprintf.c: make '%pD' print the full path
> of file
>
> On Tue, Jun 22, 2021 at 10:06:32PM +0800, Jia He wrote:
> > Previously, the specifier '%pD' is for printing dentry name of struct
> > file. It may not be perfect (by default it only prints one component.)
> >
> > As suggested by Linus [1]:
>
> Citing is better looked when you shift right it by two white spaces.
Okay, I plan to cite it with "> "
>
> > A dentry has a parent, but at the same time, a dentry really does
> > inherently have "one name" (and given just the dentry pointers, you
> > can't show mount-related parenthood, so in many ways the "show just
> > one name" makes sense for "%pd" in ways it doesn't necessarily for
> > "%pD"). But while a dentry arguably has that "one primary component",
> > a _file_ is certainly not exclusively about that last component.
> >
> > Hence change the behavior of '%pD' to print the full path of that file.
> >
> > Precision is never going to be used with %p (or any of its kernel
> > extensions) if -Wformat is turned on.
>
> > Link: https://lore.kernel.org/lkml/CAHk-=wimsMqGdzik187YWLb-
> [email protected]/ [1]
>
>
> >
>
> Shouldn't be blank lines in the tag block. I have an impression that I have
> commented on this already...
Okay
>
> ...
>
> > -last components. %pD does the same thing for struct file.
> > +last components. %pD prints full file path together with mount-related
>
> I guess you may also convert double space to a single one.
>
Okay
--
Cheers,
Justin (Jia He)
Hi Rasmus
> -----Original Message-----
> From: Rasmus Villemoes <[email protected]>
> Sent: Wednesday, June 23, 2021 4:52 AM
> To: Justin He <[email protected]>; Petr Mladek <[email protected]>; Steven
> Rostedt <[email protected]>; Sergey Senozhatsky
> <[email protected]>; Andy Shevchenko
> <[email protected]>; Jonathan Corbet <[email protected]>;
> Alexander Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>
> Cc: Peter Zijlstra (Intel) <[email protected]>; Eric Biggers
> <[email protected]>; Ahmed S. Darwish <[email protected]>; linux-
> [email protected]; [email protected]; linux-
> [email protected]; Matthew Wilcox <[email protected]>; Christoph
> Hellwig <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH v5 4/4] lib/test_printf.c: add test cases for '%pD'
>
> On 22/06/2021 16.06, Jia He wrote:
> > After the behaviour of specifier '%pD' is changed to print the full path
> > of struct file, the related test cases are also updated.
> >
> > Given the full path string of '%pD' is prepended from the end of the
> scratch
> > buffer, the check of "wrote beyond the nul-terminator" should be skipped
> > for '%pD'.
> >
> > Parameterize the new using_scratch_space in __test, do_test to skip the
> > test case mentioned above,
>
> I actually prefer the first suggestion of just having a file-global bool.
Yes, this is my previous proposal, but seems it is not satisfying ????.
--
Cheers,
Justin (Jia He)
>
> If and when we get other checks that need to be done selectively [e.g.
> "snprintf into a too short buffer produces a prefix of the full string",
> which also came up during this discussion but was ultimately kept]
> depending on the %<whatever> being exercised, we can add a "u32 nocheck"
> with a bunch of bits saying what to elide.
>
> Not insisting either way, just my $0.02.
>
Hi Andy
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Tuesday, June 22, 2021 10:43 PM
> To: Justin He <[email protected]>
> Cc: Petr Mladek <[email protected]>; Steven Rostedt <[email protected]>;
> Sergey Senozhatsky <[email protected]>; Rasmus Villemoes
> <[email protected]>; Jonathan Corbet <[email protected]>; Alexander
> Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Zijlstra (Intel) <[email protected]>; Eric
> Biggers <[email protected]>; Ahmed S. Darwish <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; Matthew Wilcox <[email protected]>; Christoph
> Hellwig <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH v5 0/4] make '%pD' print the full path of file
>
> On Tue, Jun 22, 2021 at 10:06:30PM +0800, Jia He wrote:
> > Background
> > ==========
> > Linus suggested printing the full path of file instead of printing
> > the components as '%pd'.
> >
> > Typically, there is no need for printk specifiers to take any real locks
> > (ie mount_lock or rename_lock). So I introduce a new helper d_path_fast
> > which is similar to d_path except it doesn't take any seqlock/spinlock.
> >
> > This series is based on Al Viro's d_path cleanup patches [1] which
> > lifted the inner lockless loop into a new helper.
> >
> > Link: https://lkml.org/lkml/2021/5/18/1260 [1]
> >
> > Test
> > ====
> > The cases I tested:
> > 1. print '%pD' with full path of ext4 file
> > 2. mount a ext4 filesystem upon a ext4 filesystem, and print the file
> > with '%pD'
> > 3. all test_print selftests, including the new '%14pD' '%-14pD'
>
> > 4. kasnprintf
>
> I believe you are talking about kasprintf().
>
>
> > Changelog
> > =========
> > v5:
> > - remove the RFC tag
>
> JFYI, when we drop RFC we usually start the series from v1.
>
> > - refine the commit msg/comments(by Petr, Andy)
> > - make using_scratch_space a new parameter of the test case
>
> Thanks for the update, I have found few minor things, please address them
> and
> feel free to add
> Reviewed-by: Andy Shevchenko <[email protected]>
>
I assume I can add your R-b to patch 4/4 "add test cases for '%pD'" instead of
whole series, right?
--
Cheers,
Justin (Jia He)
On Wed, Jun 23, 2021 at 02:02:45AM +0000, Justin He wrote:
> > From: Andy Shevchenko <[email protected]>
> > Sent: Tuesday, June 22, 2021 10:37 PM
> > On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote:
...
> > > * prepend_name - prepend a pathname in front of current buffer pointer
> > > - * @buffer: buffer pointer
> > > - * @buflen: allocated length of the buffer
> > > + * @p: prepend buffer which contains buffer pointer and allocated length
> >
> > > * @name: name string and length qstr structure
> >
> > Indentation issue btw, can be fixed in the same patch.
>
> Okay
> >
> > > *
> > > * With RCU path tracing, it may race with d_move(). Use READ_ONCE() to
> >
> > Shouldn't this be a separate change with corresponding Fixes tag?
>
> Sorry, I don't quite understand here.
> What do you want to fix?
Kernel doc. The Fixes tag should correspond to the changes that missed the
update of kernel doc.
--
With Best Regards,
Andy Shevchenko
On Wed, Jun 23, 2021 at 04:13:03AM +0000, Justin He wrote:
> > From: Andy Shevchenko <[email protected]>
> > Sent: Tuesday, June 22, 2021 10:43 PM
> > On Tue, Jun 22, 2021 at 10:06:30PM +0800, Jia He wrote:
...
> > > v5:
> > > - remove the RFC tag
> >
> > JFYI, when we drop RFC we usually start the series from v1.
> >
> > > - refine the commit msg/comments(by Petr, Andy)
> > > - make using_scratch_space a new parameter of the test case
> >
> > Thanks for the update, I have found few minor things, please address them
> > and
> > feel free to add
> > Reviewed-by: Andy Shevchenko <[email protected]>
> >
>
> I assume I can add your R-b to patch 4/4 "add test cases for '%pD'" instead of
> whole series, right?
It was against cover letter, means to cover the whole series, but since you do
not address my comments, do not apply to the patches we have not settled down
on.
--
With Best Regards,
Andy Shevchenko
Hi Andy
> -----Original Message-----
> From: Andy Shevchenko <[email protected]>
> Sent: Wednesday, June 23, 2021 5:07 PM
> To: Justin He <[email protected]>
> Cc: Petr Mladek <[email protected]>; Steven Rostedt <[email protected]>;
> Sergey Senozhatsky <[email protected]>; Rasmus Villemoes
> <[email protected]>; Jonathan Corbet <[email protected]>; Alexander
> Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> foundation.org>; Peter Zijlstra (Intel) <[email protected]>; Eric
> Biggers <[email protected]>; Ahmed S. Darwish <[email protected]>;
> [email protected]; [email protected]; linux-
> [email protected]; Matthew Wilcox <[email protected]>; Christoph
> Hellwig <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH v5 1/4] fs: introduce helper d_path_unsafe()
>
> On Wed, Jun 23, 2021 at 02:02:45AM +0000, Justin He wrote:
> > > From: Andy Shevchenko <[email protected]>
> > > Sent: Tuesday, June 22, 2021 10:37 PM
> > > On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote:
>
> ...
>
> > > > * prepend_name - prepend a pathname in front of current buffer
> pointer
> > > > - * @buffer: buffer pointer
> > > > - * @buflen: allocated length of the buffer
> > > > + * @p: prepend buffer which contains buffer pointer and allocated
> length
> > >
> > > > * @name: name string and length qstr structure
> > >
> > > Indentation issue btw, can be fixed in the same patch.
> >
> > Okay
> > >
> > > > *
> > > > * With RCU path tracing, it may race with d_move(). Use READ_ONCE()
> to
> > >
> > > Shouldn't this be a separate change with corresponding Fixes tag?
> >
> > Sorry, I don't quite understand here.
> > What do you want to fix?
>
> Kernel doc. The Fixes tag should correspond to the changes that missed the
> update of kernel doc.
Ah, I got your point.
Actually, this is originated from an unmerged patch [1]
[1] https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/commit/?h=work.d_path&id=ad08ae586586ea9e2c0228a3d5a083500ea54202
I will ping Al Viro to fix this
--
Cheers,
Justin (Jia He)
On Wed 2021-06-23 03:14:33, Justin He wrote:
> Hi Andy
>
> > -----Original Message-----
> > From: Andy Shevchenko <[email protected]>
> > Sent: Tuesday, June 22, 2021 10:40 PM
> > To: Justin He <[email protected]>
> > Cc: Petr Mladek <[email protected]>; Steven Rostedt <[email protected]>;
> > Sergey Senozhatsky <[email protected]>; Rasmus Villemoes
> > <[email protected]>; Jonathan Corbet <[email protected]>; Alexander
> > Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> > foundation.org>; Peter Zijlstra (Intel) <[email protected]>; Eric
> > Biggers <[email protected]>; Ahmed S. Darwish <[email protected]>;
> > [email protected]; [email protected]; linux-
> > [email protected]; Matthew Wilcox <[email protected]>; Christoph
> > Hellwig <[email protected]>; nd <[email protected]>
> > Subject: Re: [PATCH v5 2/4] lib/vsprintf.c: make '%pD' print the full path
> > of file
> >
> > On Tue, Jun 22, 2021 at 10:06:32PM +0800, Jia He wrote:
> > > Previously, the specifier '%pD' is for printing dentry name of struct
> > > file. It may not be perfect (by default it only prints one component.)
> > >
> > > As suggested by Linus [1]:
> >
> > Citing is better looked when you shift right it by two white spaces.
>
> Okay, I plan to cite it with "> "
My understanding is that Andy suggested to omit '>' and prefix it by
plain two spaces " ". It would look better to me as well.
Best Regards,
Petr
Hi Petr
> -----Original Message-----
> From: Petr Mladek <[email protected]>
> Sent: Thursday, June 24, 2021 4:47 PM
> To: Justin He <[email protected]>
> Cc: Andy Shevchenko <[email protected]>; Steven Rostedt
> <[email protected]>; Sergey Senozhatsky <[email protected]>;
> Rasmus Villemoes <[email protected]>; Jonathan Corbet
> <[email protected]>; Alexander Viro <[email protected]>; Linus Torvalds
> <[email protected]>; Peter Zijlstra (Intel)
> <[email protected]>; Eric Biggers <[email protected]>; Ahmed S.
> Darwish <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Matthew Wilcox
> <[email protected]>; Christoph Hellwig <[email protected]>; nd
> <[email protected]>
> Subject: Re: [PATCH v5 2/4] lib/vsprintf.c: make '%pD' print the full path
> of file
>
> On Wed 2021-06-23 03:14:33, Justin He wrote:
> > Hi Andy
> >
> > > -----Original Message-----
> > > From: Andy Shevchenko <[email protected]>
> > > Sent: Tuesday, June 22, 2021 10:40 PM
> > > To: Justin He <[email protected]>
> > > Cc: Petr Mladek <[email protected]>; Steven Rostedt
> <[email protected]>;
> > > Sergey Senozhatsky <[email protected]>; Rasmus Villemoes
> > > <[email protected]>; Jonathan Corbet <[email protected]>; Alexander
> > > Viro <[email protected]>; Linus Torvalds <torvalds@linux-
> > > foundation.org>; Peter Zijlstra (Intel) <[email protected]>; Eric
> > > Biggers <[email protected]>; Ahmed S. Darwish
> <[email protected]>;
> > > [email protected]; [email protected]; linux-
> > > [email protected]; Matthew Wilcox <[email protected]>;
> Christoph
> > > Hellwig <[email protected]>; nd <[email protected]>
> > > Subject: Re: [PATCH v5 2/4] lib/vsprintf.c: make '%pD' print the full
> path
> > > of file
> > >
> > > On Tue, Jun 22, 2021 at 10:06:32PM +0800, Jia He wrote:
> > > > Previously, the specifier '%pD' is for printing dentry name of struct
> > > > file. It may not be perfect (by default it only prints one component.)
> > > >
> > > > As suggested by Linus [1]:
> > >
> > > Citing is better looked when you shift right it by two white spaces.
> >
> > Okay, I plan to cite it with "> "
>
> My understanding is that Andy suggested to omit '>' and prefix it by
> plain two spaces " ". It would look better to me as well.
Okay, got it
--
Cheers,
Justin (Jia He)
On Tue 2021-06-22 17:36:39, Andy Shevchenko wrote:
> On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote:
> > This helper is similar to d_path() except that it doesn't take any
> > seqlock/spinlock. It is typical for debugging purposes. Besides,
> > an additional return value *prenpend_len* is used to get the full
> > path length of the dentry, ingoring the tail '\0'.
> > the full path length = end - buf - prepend_length - 1
>
> Missed period at the end of sentence.
>
> > Previously it will skip the prepend_name() loop at once in
> > __prepen_path() when the buffer length is not enough or even negative.
> > prepend_name_with_len() will get the full length of dentry name
> > together with the parent recursively regardless of the buffer length.
>
> > If someone invokes snprintf() with small but positive space,
> > prepend_name_with_len() moves and copies the string partially.
> >
> > More than that, kasprintf() will pass NULL _buf_ and _end_ as the
> > parameters. Hence return at the very beginning with false in this case.
>
> These two paragraphs are talking about printf() interface, while patch has
> nothing to do with it. Please, rephrase in a way that it doesn't refer to the
> particular callers. Better to mention them in the corresponding printf()
> patch(es).
The two paragraphs are actually repeated in the 2nd
patch. Unfortunately, they do not make sense there either because they
comment code that is modified in this patch.
We could describe it here a generic way. For example:
prepend_name_with_len() moves and copies the path when the given
buffer is not big enough. It cuts off the end of the path.
It returns immediately when there is no buffer at all.
Best Regards,
Petr
On Thu, Jun 24, 2021 at 11:26:53AM +0200, Petr Mladek wrote:
> On Tue 2021-06-22 17:36:39, Andy Shevchenko wrote:
> > On Tue, Jun 22, 2021 at 10:06:31PM +0800, Jia He wrote:
> > > This helper is similar to d_path() except that it doesn't take any
> > > seqlock/spinlock. It is typical for debugging purposes. Besides,
> > > an additional return value *prenpend_len* is used to get the full
> > > path length of the dentry, ingoring the tail '\0'.
> > > the full path length = end - buf - prepend_length - 1
> >
> > Missed period at the end of sentence.
> >
> > > Previously it will skip the prepend_name() loop at once in
> > > __prepen_path() when the buffer length is not enough or even negative.
> > > prepend_name_with_len() will get the full length of dentry name
> > > together with the parent recursively regardless of the buffer length.
> >
> > > If someone invokes snprintf() with small but positive space,
> > > prepend_name_with_len() moves and copies the string partially.
> > >
> > > More than that, kasprintf() will pass NULL _buf_ and _end_ as the
> > > parameters. Hence return at the very beginning with false in this case.
> >
> > These two paragraphs are talking about printf() interface, while patch has
> > nothing to do with it. Please, rephrase in a way that it doesn't refer to the
> > particular callers. Better to mention them in the corresponding printf()
> > patch(es).
>
> The two paragraphs are actually repeated in the 2nd
> patch. Unfortunately, they do not make sense there either because they
> comment code that is modified in this patch.
>
> We could describe it here a generic way. For example:
>
> prepend_name_with_len() moves and copies the path when the given
> buffer is not big enough. It cuts off the end of the path.
> It returns immediately when there is no buffer at all.
Yes, that's my point, but sorry if I made it unclear.
--
With Best Regards,
Andy Shevchenko