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 <[email protected]> Co-developed-by: Claude Sonnet 4.5 <[email protected]> Signed-off-by: Ahmad Fatoum <[email protected]> --- 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
