From 9ae3638429f253c5947bfb57129338122e77fa51 Mon Sep 17 00:00:00 2001 From: Juan RP Date: Thu, 19 Feb 2015 11:04:34 +0100 Subject: [PATCH] Stop converting relative symlinks to absolute. There's no reason to make them absolute, simply store in the metadata the target file as is. This vastly simplifies the code and makes all test pass correctly. --- NEWS | 4 ++- bin/xbps-create/main.c | 48 ++-------------------------- bin/xbps-pkgdb/check_pkg_symlinks.c | 2 +- include/xbps.h.in | 13 +++----- lib/package_remove.c | 2 +- lib/util.c | 48 ++-------------------------- tests/xbps/xbps-create/basic_test.sh | 8 ++--- 7 files changed, 20 insertions(+), 105 deletions(-) diff --git a/NEWS b/NEWS index eb28c263..3c9bb542 100644 --- a/NEWS +++ b/NEWS @@ -3,7 +3,9 @@ xbps-0.44 (???): * xbps-pkgdb(8): this now uses the code from libxbps to check the target file of symlinks. This fixes some false positives with symlinks. - * xbps-create(8): properly detect target of relative symlinks in some cases. + * xbps-create(8): relative target file of symlinks aren't converted to absolute + anymore. This just stores what the symlink points to, does not matter if + it's relative or absolute path. * libxbps: make sure that symlinks with relative target are detected and removed properly. Fix #78: https://github.com/voidlinux/xbps/issues/78 diff --git a/bin/xbps-create/main.c b/bin/xbps-create/main.c index e089700d..8c47bc64 100644 --- a/bin/xbps-create/main.c +++ b/bin/xbps-create/main.c @@ -1,5 +1,5 @@ /*- - * Copyright (c) 2012-2014 Juan Romero Pardines. + * Copyright (c) 2012-2015 Juan Romero Pardines. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -234,8 +234,6 @@ ftw_cb(const char *fpath, const struct stat *sb, int type, struct FTW *ftwbuf _u { struct xentry *xe = NULL; const char *filep = NULL; - char *buf, *p, *p2, *dname; - ssize_t r; /* Ignore metadata files generated by xbps-src and destdir */ if ((strcmp(fpath, ".") == 0) || @@ -272,50 +270,10 @@ ftw_cb(const char *fpath, const struct stat *sb, int type, struct FTW *ftwbuf _u */ xe->type = strdup("links"); assert(xe->type); - buf = malloc(sb->st_size+1); - assert(buf); - r = readlink(fpath, buf, sb->st_size+1); - if (r < 0 || r > sb->st_size) + xe->target = xbps_symlink_target(fpath); + if (xe->target == NULL) die("failed to process symlink %s:", fpath); - buf[sb->st_size] = '\0'; - /* - * Check if symlink is absolute or relative; on the former - * make it absolute for the target object. - */ - if (strstr(buf, "./")) { - p = realpath(fpath, NULL); - if (p == NULL) { - /* - * This symlink points to an unexistent file, - * which might be provided in another package. - * So let's use the same target. - */ - xe->target = strdup(buf); - } else { - /* - * Sanitize destdir just in case. - */ - if ((p2 = realpath(destdir, NULL)) == NULL) - die("failed to sanitize destdir %s: %s", destdir, strerror(errno)); - - xe->target = strdup(p+strlen(p2)); - free(p2); - free(p); - } - } else if (buf[0] != '/') { - /* relative path */ - p = strdup(filep); - assert(p); - dname = dirname(p); - assert(dname); - xe->target = xbps_xasprintf("%s/%s", dname, buf); - free(p); - } else { - xe->target = strdup(buf); - } - assert(xe->target); - free(buf); } else if (type == FTW_F) { struct xentry *xep; bool hlink = false; diff --git a/bin/xbps-pkgdb/check_pkg_symlinks.c b/bin/xbps-pkgdb/check_pkg_symlinks.c index 1bbbad93..94c758d4 100644 --- a/bin/xbps-pkgdb/check_pkg_symlinks.c +++ b/bin/xbps-pkgdb/check_pkg_symlinks.c @@ -77,7 +77,7 @@ check_pkg_symlinks(struct xbps_handle *xhp, const char *pkgname, void *arg) continue; } snprintf(path, sizeof(path), "%s/%s", xhp->rootdir, file); - if ((lnk = xbps_symlink_target(xhp, path, tgt)) == NULL) { + if ((lnk = xbps_symlink_target(path)) == NULL) { xbps_error_printf("%s: broken symlink %s (target: %s)\n", pkgname, file, tgt); broken = true; continue; diff --git a/include/xbps.h.in b/include/xbps.h.in index 23dea55b..12775a47 100644 --- a/include/xbps.h.in +++ b/include/xbps.h.in @@ -48,7 +48,7 @@ * * This header documents the full API for the XBPS Library. */ -#define XBPS_API_VERSION "20150219-1" +#define XBPS_API_VERSION "20150219-2" #ifndef XBPS_VERSION #define XBPS_VERSION "UNSET" @@ -1945,17 +1945,14 @@ char *xbps_pubkey2fp(struct xbps_handle *xhp, xbps_data_t pubkey); char *xbps_sanitize_path(const char *src); /** - * Returns a sanitized target file of \a path without the rootdir component. + * Returns the target file of the symlink \a symlink. * - * @param[in] xhp The pointer to an xbps_handle struct. - * @param[in] path path component. - * @param[in] target The stored target file of a symlink. + * @param[in] symlink The path to a symlink. * - * @return The sanitized path in a buffer. + * @return The target file in a buffer. * The returned buffer must be free(3)d when it's no longer necessary. */ -char *xbps_symlink_target(struct xbps_handle *xhp, const char *path, - const char *target); +char *xbps_symlink_target(const char *symlink); /*@}*/ diff --git a/lib/package_remove.c b/lib/package_remove.c index fff1c9d3..4fdfc6f2 100644 --- a/lib/package_remove.c +++ b/lib/package_remove.c @@ -213,7 +213,7 @@ remove_pkg_files(struct xbps_handle *xhp, xbps_dictionary_get_cstring_nocopy(obj, "target", &target); assert(target); - lnk = xbps_symlink_target(xhp, path, target); + lnk = xbps_symlink_target(path); if (lnk == NULL) { xbps_dbg_printf(xhp, "[remove] %s " "symlink_target: %s\n", path, strerror(errno)); diff --git a/lib/util.c b/lib/util.c index 066d0a7c..e9f84324 100644 --- a/lib/util.c +++ b/lib/util.c @@ -454,10 +454,10 @@ xbps_sanitize_path(const char *src) } char * -xbps_symlink_target(struct xbps_handle *xhp, const char *path, const char *tgt) +xbps_symlink_target(const char *path) { struct stat sb; - char *p, *p1, *dname, *res = NULL, *lnk = NULL; + char *lnk = NULL; ssize_t r; if (lstat(path, &sb) == -1) @@ -472,47 +472,5 @@ xbps_symlink_target(struct xbps_handle *xhp, const char *path, const char *tgt) return NULL; } lnk[sb.st_size] = '\0'; - - if (tgt[0] != '/') { - /* - * target file is relative and wasn't converted to absolute by - * xbps-create(8), just compare it as is. - */ - return lnk; - } - - if (strstr(lnk, "./") || lnk[0] != '/') { - /* relative */ - p = strdup(path); - assert(p); - dname = dirname(p); - assert(dname); - p = xbps_xasprintf("%s/%s", dname, lnk); - assert(p); - p1 = xbps_sanitize_path(p); - assert(p1); - free(p); - if ((strstr(p1, "./")) && (p = realpath(p1, NULL))) { - if (strcmp(xhp->rootdir, "/") == 0) - res = strdup(p); - else - res = strdup(p + strlen(xhp->rootdir)); - - free(p); - } - if (res == NULL) { - if (strcmp(xhp->rootdir, "/") == 0) - res = strdup(p1); - else - res = strdup(p1 + strlen(xhp->rootdir)); - } - assert(res); - free(lnk); - free(p1); - } else { - /* absolute */ - res = lnk; - } - - return res; + return lnk; } diff --git a/tests/xbps/xbps-create/basic_test.sh b/tests/xbps/xbps-create/basic_test.sh index 34600889..493d6e24 100644 --- a/tests/xbps/xbps-create/basic_test.sh +++ b/tests/xbps/xbps-create/basic_test.sh @@ -33,7 +33,7 @@ hardlinks_size_body() { atf_test_case symlink_relative_target symlink_relative_target_head() { - atf_set "descr" "xbps-create(8): relative symlinks in destdir must be absolute" + atf_set "descr" "xbps-create(8): relative symlinks in destdir" } symlink_relative_target_body() { @@ -48,7 +48,7 @@ symlink_relative_target_body() { xbps-rindex -d -a repo/*.xbps atf_check_equal $? 0 result="$(xbps-query -r root --repository=repo -f foo|tr -d '\n')" - expected="/usr/include/gsm/gsm.h/usr/include/gsm.h -> /usr/include/gsm/gsm.h" + expected="/usr/include/gsm/gsm.h/usr/include/gsm.h -> gsm/gsm.h" rv=0 if [ "$result" != "$expected" ]; then echo "result: $result" @@ -61,7 +61,7 @@ symlink_relative_target_body() { atf_test_case symlink_relative_target_cwd symlink_relative_target_cwd_head() { - atf_set "descr" "xbps-create(8): relative symlinks to cwd in destdir must be absolute" + atf_set "descr" "xbps-create(8): relative symlinks to cwd in destdir" } symlink_relative_target_cwd_body() { @@ -76,7 +76,7 @@ symlink_relative_target_cwd_body() { xbps-rindex -d -a repo/*.xbps atf_check_equal $? 0 result="$(xbps-query -r root --repository=repo -f foo|tr -d '\n')" - expected="/usr/include/gsm/gsm.h/usr/include/gsm.h -> /usr/include/gsm/gsm.h" + expected="/usr/include/gsm/gsm.h/usr/include/gsm.h -> ./gsm/gsm.h" rv=0 if [ "$result" != "$expected" ]; then echo "result: $result"