[PATCH] commands: readlink: restore support for non-existent last component

[PATCH] commands: readlink: restore support for non-existent last component

[Ahmad Fatoum]

The documentation and commit a6f379599227 ("readlink: Improve -f handling") hint
that the last component missing used to not be an error. This is not the
case currently, so restore the original behavior and add an -e option
for the current behavior of all components existing.

To ensure this doesn't regress in future, also add some AI-generated
test cases.

Cc: Ian Abbott <abbotti@mev.co.uk>
Co-developed-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/Kconfig               |   7 +-
 commands/readlink.c            |  66 ++++++++---
 test/py/test_shell_readlink.py | 202 +++++++++++++++++++++++++++++++++
 3 files changed, 256 insertions(+), 19 deletions(-)
 create mode 100644 test/py/test_shell_readlink.py

diff --git a/commands/Kconfig b/commands/Kconfig
index 297c89b4b566..e7de951005f4 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -1161,14 +1161,17 @@ config CMD_READLINK
 	help
 	  Read value of a symbolic link
 
-	  Usage: readlink [-f] FILE [VARIABLE]
+	  Usage: readlink [-fe] FILE [VARIABLE]
 
 	  Read value of a symbolic link or canonical file name
 	  The value is either stored it into the specified VARIABLE
 	  or printed.
 
 	  Options:
-		  -f	canonicalize by following first symlink
+		-f	canonicalize by following symlinks
+			final component need not exist
+		-e	canonicalize by following symlinks
+			all components must exist
 
 config CMD_RM
 	tristate
diff --git a/commands/readlink.c b/commands/readlink.c
index a37b6d7512d7..021fbaa3e08b 100644
--- a/commands/readlink.c
+++ b/commands/readlink.c
@@ -11,6 +11,8 @@
 #include <malloc.h>
 #include <getopt.h>
 
+enum can_mode { CAN_NONE, CAN_ALL_BUT_LAST, CAN_EXISTING };
+
 static void output_result(const char *var, const char *val)
 {
 	if (var)
@@ -20,20 +22,59 @@ static void output_result(const char *var, const char *val)
 
 }
 
+static int canonicalize_filename_mode(const char *var, char *path, enum can_mode can_mode)
+{
+	char *buf = NULL, *file, *dir;
+	struct stat s;
+	int ret = -1;
+
+	buf = canonicalize_path(AT_FDCWD, path);
+	if (buf)
+		goto out;
+
+	switch (can_mode) {
+	case CAN_ALL_BUT_LAST:
+		file = basename(path);
+		dir = dirname(path);
+
+		buf = canonicalize_path(AT_FDCWD, dir);
+		if (!buf || stat(dir, &s) || !S_ISDIR(s.st_mode))
+			goto err;
+
+		buf = xrasprintf(buf, "/%s", file);
+		break;
+	case CAN_EXISTING:
+	case CAN_NONE:
+		goto err;
+	}
+
+out:
+	output_result(var, buf);
+	ret = 0;
+err:
+	free(buf);
+	return ret;
+}
+
 static int do_readlink(int argc, char *argv[])
 {
 	const char *var;
 	char *path, realname[PATH_MAX];
-	int canonicalize = 0;
+	enum can_mode can_mode = CAN_NONE;
 	int opt;
 
 	memset(realname, 0, PATH_MAX);
 
-	while ((opt = getopt(argc, argv, "f")) > 0) {
+	while ((opt = getopt(argc, argv, "fe")) > 0) {
 		switch (opt) {
 		case 'f':
-			canonicalize = 1;
+			can_mode = CAN_ALL_BUT_LAST;
 			break;
+		case 'e':
+			can_mode = CAN_EXISTING;
+			break;
+		default:
+			return COMMAND_ERROR_USAGE;
 		}
 	}
 
@@ -46,18 +87,9 @@ static int do_readlink(int argc, char *argv[])
 	path = argv[0];
 	var = argv[1];
 
-	if (canonicalize) {
-		char *buf = canonicalize_path(AT_FDCWD, path);
-		struct stat s;
-
-		if (!buf)
+	if (can_mode > CAN_NONE) {
+		if (canonicalize_filename_mode(var, path, can_mode))
 			goto err;
-		if (stat(dirname(path), &s) || !S_ISDIR(s.st_mode)) {
-			free(buf);
-			goto err;
-		}
-		output_result(var, buf);
-		free(buf);
 	} else {
 		if (readlink(path, realname, PATH_MAX - 1) < 0)
 			goto err;
@@ -77,14 +109,14 @@ BAREBOX_CMD_HELP_TEXT("The value is either stored it into the specified VARIABLE
 BAREBOX_CMD_HELP_TEXT("or printed.")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-f", "canonicalize by following symlinks;")
-BAREBOX_CMD_HELP_OPT("",   "final component need not exist");
+BAREBOX_CMD_HELP_OPT("-f", "canonicalize by following symlinks; final component need not exist")
+BAREBOX_CMD_HELP_OPT("-e", "canonicalize by following symlinks; all components must exist")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(readlink)
 	.cmd		= do_readlink,
 	BAREBOX_CMD_DESC("read value of a symbolic link or canonical file name")
-	BAREBOX_CMD_OPTS("[-f] FILE [VARIABLE]")
+	BAREBOX_CMD_OPTS("[-fe] FILE [VARIABLE]")
 	BAREBOX_CMD_GROUP(CMD_GRP_FILE)
 	BAREBOX_CMD_HELP(cmd_readlink_help)
 BAREBOX_CMD_END
diff --git a/test/py/test_shell_readlink.py b/test/py/test_shell_readlink.py
new file mode 100644
index 000000000000..ccd8ccfec421
--- /dev/null
+++ b/test/py/test_shell_readlink.py
@@ -0,0 +1,202 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+from .helper import skip_disabled
+
+
+def test_cmd_readlink(barebox, barebox_config):
+    skip_disabled(barebox_config, "CONFIG_CMD_READLINK", "CONFIG_CMD_MKDIR")
+
+    # Create test directory structure
+    barebox.run_check('mkdir -p /tmp/readlink-test/subdir')
+    barebox.run_check('cd /tmp/readlink-test')
+
+    stdout = barebox.run_check('readlink -f .')
+    assert stdout == ["/tmp/readlink-test"]
+
+    # Test relative path resolution
+    stdout = barebox.run_check('readlink -f ./subdir')
+    assert stdout == ["/tmp/readlink-test/subdir"]
+
+    # Test with variable storage
+    stdout = barebox.run_check('readlink -f ./subdir mypath && echo $mypath')
+    assert stdout == ["/tmp/readlink-test/subdir"]
+
+    stdout = barebox.run_check('readlink -f ./subdir/something mypath && echo $mypath')
+    assert stdout == ["/tmp/readlink-test/subdir/something"]
+
+    # Test with variable storage (positional argument)
+    stdout = barebox.run_check('readlink -f subdir mypath && echo $mypath')
+    assert stdout == ["/tmp/readlink-test/subdir"]
+
+    # Test with absolute path (should return as-is)
+    stdout = barebox.run_check('readlink -f /tmp')
+    assert stdout == ["/tmp"]
+
+    # Cleanup
+    barebox.run_check('rm -r /tmp/readlink-test')
+
+
+def test_cmd_readlink_nonexistent(barebox, barebox_config):
+    skip_disabled(barebox_config, "CONFIG_CMD_READLINK", "CONFIG_CMD_MKDIR")
+
+    # Create test directory structure
+    barebox.run_check('mkdir -p /tmp/readlink-test2/existing')
+    barebox.run_check('cd /tmp/readlink-test2')
+
+    # Test -f with non-existent final component (should work)
+    stdout = barebox.run_check('readlink -f existing/nonexistent')
+    assert stdout == ["/tmp/readlink-test2/existing/nonexistent"]
+
+    # Test -f with nested non-existent final component
+    stdout = barebox.run_check('readlink -f ./existing/also-nonexistent')
+    assert stdout == ["/tmp/readlink-test2/existing/also-nonexistent"]
+
+    # Test -e with non-existent final component (should fail)
+    _, _, returncode = barebox.run('readlink -e existing/nonexistent')
+    assert returncode != 0
+
+    # Test -e with existing component (should work)
+    stdout = barebox.run_check('readlink -e existing')
+    assert stdout == ["/tmp/readlink-test2/existing"]
+
+    # Test -f with non-existent parent directory (should fail)
+    _, _, returncode = barebox.run('readlink -f nonexistent-parent/file')
+    assert returncode != 0
+
+    # Test -e with non-existent parent directory (should fail)
+    _, _, returncode = barebox.run('readlink -e nonexistent-parent/file')
+    assert returncode != 0
+
+    # Cleanup
+    barebox.run_check('rm -r /tmp/readlink-test2')
+
+
+def test_cmd_readlink_symlinks(barebox, barebox_config):
+    skip_disabled(barebox_config, "CONFIG_CMD_READLINK", "CONFIG_CMD_MKDIR", "CONFIG_CMD_LN", "CONFIG_CMD_ECHO")
+
+    # Create test directory structure with symlinks
+    barebox.run_check('mkdir -p /tmp/readlink-test3/real/deep')
+    barebox.run_check('cd /tmp/readlink-test3')
+    barebox.run_check('echo -o real/file.txt content')
+
+    # Create various symlinks
+    barebox.run_check('ln real link-to-real')
+    barebox.run_check('ln real/file.txt link-to-file')
+    barebox.run_check('ln link-to-real link-to-link')
+    barebox.run_check('ln nonexistent broken-link')
+
+    # Test basic readlink (no -f/-e, just read symlink value)
+    stdout = barebox.run_check('readlink link-to-real')
+    assert stdout == ["real"]
+
+    stdout = barebox.run_check('readlink link-to-file')
+    assert stdout == ["real/file.txt"]
+
+    # Test -f following single symlink
+    stdout = barebox.run_check('readlink -f link-to-real')
+    assert stdout == ["/tmp/readlink-test3/real"]
+
+    stdout = barebox.run_check('readlink -f link-to-file')
+    assert stdout == ["/tmp/readlink-test3/real/file.txt"]
+
+    # Test -f following multiple levels of symlinks
+    stdout = barebox.run_check('readlink -f link-to-link')
+    assert stdout == ["/tmp/readlink-test3/real"]
+
+    # Test -e following symlinks (should work when target exists)
+    stdout = barebox.run_check('readlink -e link-to-real')
+    assert stdout == ["/tmp/readlink-test3/real"]
+
+    stdout = barebox.run_check('readlink -e link-to-file')
+    assert stdout == ["/tmp/readlink-test3/real/file.txt"]
+
+    # Test -e with broken symlink (should fail)
+    _, _, returncode = barebox.run('readlink -e broken-link')
+    assert returncode != 0
+
+    # Test -f with broken symlink (canonicalizes the link path itself)
+    stdout = barebox.run_check('readlink -f broken-link')
+    assert stdout == ["/tmp/readlink-test3/broken-link"]
+
+    # Test -f with symlink to non-existent file in existing subdirectory
+    barebox.run_check('ln real/nonexistent link-to-nonexistent')
+    stdout = barebox.run_check('readlink -f link-to-nonexistent')
+    assert stdout == ["/tmp/readlink-test3/link-to-nonexistent"]
+
+    # Test -e with symlink to non-existent file (should fail)
+    _, _, returncode = barebox.run('readlink -e link-to-nonexistent')
+    assert returncode != 0
+
+    # Cleanup
+    barebox.run_check('rm -r /tmp/readlink-test3')
+
+
+def test_cmd_readlink_parent_refs(barebox, barebox_config):
+    skip_disabled(barebox_config, "CONFIG_CMD_READLINK", "CONFIG_CMD_MKDIR")
+
+    # Create test directory structure
+    barebox.run_check('mkdir -p /tmp/readlink-test4/a/b/c')
+    barebox.run_check('cd /tmp/readlink-test4/a/b/c')
+
+    # Test parent directory references with -f
+    stdout = barebox.run_check('readlink -f ..')
+    assert stdout == ["/tmp/readlink-test4/a/b"]
+
+    stdout = barebox.run_check('readlink -f ../..')
+    assert stdout == ["/tmp/readlink-test4/a"]
+
+    stdout = barebox.run_check('readlink -f ../../..')
+    assert stdout == ["/tmp/readlink-test4"]
+
+    # Test parent with additional path components
+    stdout = barebox.run_check('readlink -f ../../../a/b')
+    assert stdout == ["/tmp/readlink-test4/a/b"]
+
+    # Test -f with non-existent path using parent refs
+    stdout = barebox.run_check('readlink -f ../../nonexistent')
+    assert stdout == ["/tmp/readlink-test4/a/nonexistent"]
+
+    # Test -e with parent references (existing path)
+    stdout = barebox.run_check('readlink -e ..')
+    assert stdout == ["/tmp/readlink-test4/a/b"]
+
+    # Test -e with parent references (non-existent final component)
+    _, _, returncode = barebox.run('readlink -e ../../nonexistent')
+    assert returncode != 0
+
+    # Cleanup
+    barebox.run_check('rm -r /tmp/readlink-test4')
+
+
+def test_cmd_readlink_edge_cases(barebox, barebox_config):
+    skip_disabled(barebox_config, "CONFIG_CMD_READLINK", "CONFIG_CMD_MKDIR")
+
+    # Create test directory structure
+    barebox.run_check('mkdir -p /tmp/readlink-test5/dir')
+    barebox.run_check('cd /tmp/readlink-test5')
+
+    # Test with trailing slash on existing directory
+    stdout = barebox.run_check('readlink -f dir/')
+    assert stdout == ["/tmp/readlink-test5/dir"]
+
+    # Test with multiple slashes
+    stdout = barebox.run_check('readlink -f ./dir//subpath')
+    assert stdout == ["/tmp/readlink-test5/dir/subpath"]
+
+    # Test self-reference
+    stdout = barebox.run_check('readlink -f ./././dir')
+    assert stdout == ["/tmp/readlink-test5/dir"]
+
+    # Test -e with trailing slash on existing directory
+    stdout = barebox.run_check('readlink -e dir/')
+    assert stdout == ["/tmp/readlink-test5/dir"]
+
+    # Test absolute path that's already canonical
+    stdout = barebox.run_check('readlink -f /tmp/readlink-test5/dir')
+    assert stdout == ["/tmp/readlink-test5/dir"]
+
+    stdout = barebox.run_check('readlink -e /tmp/readlink-test5/dir')
+    assert stdout == ["/tmp/readlink-test5/dir"]
+
+    # Cleanup
+    barebox.run_check('rm -r /tmp/readlink-test5')
-- 
2.47.3

Re: [PATCH] commands: readlink: restore support for non-existent last component

[Sascha Hauer]

On Mon, 26 Jan 2026 11:52:33 +0100, Ahmad Fatoum wrote:
> The documentation and commit a6f379599227 ("readlink: Improve -f handling") hint
> that the last component missing used to not be an error. This is not the
> case currently, so restore the original behavior and add an -e option
> for the current behavior of all components existing.
> 
> To ensure this doesn't regress in future, also add some AI-generated
> test cases.
> 
> [...]

Applied, thanks!

[1/1] commands: readlink: restore support for non-existent last component
      https://git.pengutronix.de/cgit/barebox/commit/?id=c2bec56be268 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>