[PATCH] fs: Fix typo in comment of link_path_walk()

[PATCH] fs: Fix typo in comment of link_path_walk()

[I Hsin Cheng]

Fix "NUL" to "NULL".

Fixes: 200e9ef7ab51 ("vfs: split up name hashing in link_path_walk() into helper function")
Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
---
 fs/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 84a0e0b0111c..a2526e2103fe 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2480,7 +2480,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 		if (!*name)
 			goto OK;
 		/*
-		 * If it wasn't NUL, we know it was '/'. Skip that
+		 * If it wasn't NULL, we know it was '/'. Skip that
 		 * slash, and continue until no more slashes.
 		 */
 		do {
-- 
2.43.0

Re: [PATCH] fs: Fix typo in comment of link_path_walk()

[Al Viro]

On Sat, May 10, 2025 at 06:46:32PM +0800, I Hsin Cheng wrote:
> Fix "NUL" to "NULL".
> 
> Fixes: 200e9ef7ab51 ("vfs: split up name hashing in link_path_walk() into helper function")

Not a typo.  And think for a second about the meaning of
so "fixed" sentence - NUL and '/' are mutually exclusive
alternatives; both are characters.  NULL is a pointer and
makes no sense whatsoever in that context.

Re: [PATCH] fs: Fix typo in comment of link_path_walk()

[I Hsin Cheng]

On Sat, May 10, 2025 at 01:08:35PM +0100, Al Viro wrote:
> On Sat, May 10, 2025 at 06:46:32PM +0800, I Hsin Cheng wrote:
> > Fix "NUL" to "NULL".
> > 
> > Fixes: 200e9ef7ab51 ("vfs: split up name hashing in link_path_walk() into helper function")
> 
> Not a typo.  And think for a second about the meaning of
> so "fixed" sentence - NUL and '/' are mutually exclusive
> alternatives; both are characters.  NULL is a pointer and
> makes no sense whatsoever in that context.

Ohh thanks for the head up. I'll keep this in mind, really big thanks!

Best regards,
I Hsin Cheng

Re: [PATCH] fs: Fix typo in comment of link_path_walk()

[Al Viro]

On Sat, May 10, 2025 at 08:44:12PM +0800, I Hsin Cheng wrote:
> On Sat, May 10, 2025 at 01:08:35PM +0100, Al Viro wrote:
> > On Sat, May 10, 2025 at 06:46:32PM +0800, I Hsin Cheng wrote:
> > > Fix "NUL" to "NULL".
> > > 
> > > Fixes: 200e9ef7ab51 ("vfs: split up name hashing in link_path_walk() into helper function")
> > 
> > Not a typo.  And think for a second about the meaning of
> > so "fixed" sentence - NUL and '/' are mutually exclusive
> > alternatives; both are characters.  NULL is a pointer and
> > makes no sense whatsoever in that context.
> 
> Ohh thanks for the head up. I'll keep this in mind, really big thanks!

FWIW, the logics could be cleaner; what happens there is that having
consumed a pathname component, we want to find two things:
	* was it the last one?
	* if not, where does the next one start?
If the next character is the end of string (NUL, zero byte), everything's
obvious - it was the last component.  If not, the next character must
be '/' (anything else would've been a part of component we've just read
through), and usually that would mean that there are other components
coming after it.  However, there might be more than one slash (/usr//bin
means the same thing as /usr/bin), so we need to skip however many slashes
there might be in order to find the beginning of the next component.
What's more, /usr/bin/ is also valid and means the same thing as /usr/bin,
so it's still possible that component we've read was the last one - we
won't know until we skip all slashes and see if we've reached the end
of string.

A possible way to clarify that might be something like

/* given a pointer just past the end of component find the next one */
static inline const char *next_component(const char *s)
{
	if (!*s)		// end of string
		return NULL;
	// the only other thing possible here is '/'; skip it, along with
	// any extra slashes (if any - rare, but legal) right after it.
	do {
		s++;
	} while (unlikely(*s == '/'));
	if (unlikely(!*s))	// slash(es), then end of string
		return NULL;
	return s;		// beginning of the next component
}

with
		name = next_component(name);
		if (!name) {
			/* we are at the end of... */
                        if (!depth) {
				/* ... the pathname or trailing symlink; done */
                                nd->dir_vfsuid = i_uid_into_vfsuid(idmap, nd->inode);
                                nd->dir_mode = nd->inode->i_mode;
                                nd->flags &= ~LOOKUP_PARENT;
                                return 0;
                        }
                        /* ... the last component of nested symlink */
                        name = nd->stack[--depth].name;
                        link = walk_component(nd, 0);
		} else {
			/* not the last component */
			link = walk_component(nd, WALK_MORE);
		}

in the corresponding place in link_path_walk().  Not sure if it's better
that way, to be honest - taking that chunk into an inline helper would
force reader to go looking for the definition of that helper and that
just might be more disruptive than the current variant.  Hell knows...

Re: [PATCH] fs: Fix typo in comment of link_path_walk()

[Matthew Wilcox]

On Sat, May 10, 2025 at 01:08:35PM +0100, Al Viro wrote:
> On Sat, May 10, 2025 at 06:46:32PM +0800, I Hsin Cheng wrote:
> > Fix "NUL" to "NULL".
> > 
> > Fixes: 200e9ef7ab51 ("vfs: split up name hashing in link_path_walk() into helper function")
> 
> Not a typo.  And think for a second about the meaning of
> so "fixed" sentence - NUL and '/' are mutually exclusive
> alternatives; both are characters.  NULL is a pointer and
> makes no sense whatsoever in that context.

fwiw, C refers to strings as being 'null terminated' rather than NUL
terminated.  eg 5.2.1:

"A byte with all bits set to 0, called the null character, shall exist
in the basic execution character set; it is used to terminate a character
string."

so NULL-terminated is incorrect because it uses the wrong case.  All this
to sau. I Hsin Cheng, please do not submit patches "correcting" comments
in the other direction (ie changing NULL to NUL).  They aren't
ambiguous.