On 23/07/12 03:30, Andrew Gregory wrote:

Resolving root early prevents later calls to realpath from having to do the work of actually resolving any symlinks in root. Also removes the unnecessary root variable.

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/query.c | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/pacman/query.c b/src/pacman/query.c index bb75465..84c9d3b 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -94,7 +94,6 @@ static int query_fileowner(alpm_list_t *targets) { int ret = 0; char path[PATH_MAX]; - const char *root; size_t rootlen; alpm_list_t *t; alpm_db_t *db_local; @@ -108,14 +107,23 @@ static int query_fileowner(alpm_list_t *targets) /* Set up our root path buffer. We only need to copy the location of root in * once, then we can just overwrite whatever file was there on the previous * iteration. */ - root = alpm_option_get_root(config->handle); - rootlen = strlen(root); - if(rootlen + 1 > PATH_MAX) { - /* we are in trouble here */ - pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, ""); + + /* resolve root now so any symlinks will only have to be resolved once */ + if(!realpath(alpm_option_get_root(config->handle), path)){ + pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"), + alpm_option_get_root(config->handle), strerror(errno));

return 1;

+ } + + /* make sure there's enough room to append the package file to path */ + rootlen = strlen(path); + if(rootlen + 2 > PATH_MAX) { + pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), path, ""); return 1; } - strcpy(path, root); + + /* append trailing '/' removed by realpath */ + path[rootlen++] = '/'; + path[rootlen] = '\0';

db_local = alpm_get_localdb(config->handle);

@@ -200,7 +208,8 @@ static int query_fileowner(alpm_list_t *targets) }

if(rootlen + 1 + strlen(pkgfile) > PATH_MAX) { - pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, pkgfile); + pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), path, pkgfile); + continue;

That continue looks familiar. Please build on top of you previous patch to avoid confusion and make applying the patch easier: https://patchwork.archlinux.org/patch/203/ (I know the confusion is mostly our fault given the number of patches floating around in this area that have not been merged yet...)

} /* concatenate our file and the root path */ strcpy(path + rootlen, pkgfile); @@ -208,8 +217,15 @@ static int query_fileowner(alpm_list_t *targets) pdname = mdirname(path); ppath = realpath(pdname, NULL); free(pdname); + path[rootlen] = '\0'; /* reset path for error messages */ + + if(!ppath) { + pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"), + ppath, strerror(errno)); + continue; + }

- if(ppath && strcmp(ppath, rpath) == 0) { + if(strcmp(ppath, rpath) == 0) { print_query_fileowner(filename, info); found = 1; }

On 23/07/12 03:30, Andrew Gregory wrote:

Trailing '/' in paths causes lstat to dereference symlinks to directories which causes it to break even though the symlink is a valid target.

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/query.c | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/src/pacman/query.c b/src/pacman/query.c index 84c9d3b..dca45e7 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -136,6 +136,12 @@ static int query_fileowner(alpm_list_t *targets)

filename = strdup(t->data);

+ /* trailing '/' causes lstat to resolve directory symlinks */ + size_t len = strlen(filename) - 1; + while(len > 0 && filename[len] == '/'){ + filename[len] = '\0'; + }

while? Are you wanting to remove multiple trailing '/' at the and of the path and forgot to add a decrement here?

+ if(lstat(filename, &buf) == -1) { /* if it is not a path but a program name, then check in PATH */ if(strchr(filename, '/') == NULL) {

On 26/07/12 15:04, Andrew Gregory wrote:

Trailing '/' in paths causes lstat to dereference symlinks to directories which causes it to break even though the symlink is a valid target.

Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/query.c | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/src/pacman/query.c b/src/pacman/query.c index 176f91c..d145707 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -137,6 +137,12 @@ static int query_fileowner(alpm_list_t *targets)

filename = strdup(t->data);

+ /* trailing '/' causes lstat to dereference directory symlinks */ + size_t len = strlen(filename) - 1;

The current style is to declare all variables at the start of the function. I made that adjustment and pushed to my working branch. Allan