[PATCH v3 1/2] pacman/query.c: in query_fileowner, make is_dir an int

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 1/2] pacman/query.c: in query_fileowner, make is_dir an int

Ivy Foster-2
From: Ivy Foster <[hidden email]>

S_ISDIR is int and "returns non-zero" if the file is a directory.

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/query.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/pacman/query.c b/src/pacman/query.c
index 4a37a338..91ca78a7 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -158,8 +158,9 @@ static int query_fileowner(alpm_list_t *targets)
  char rpath[PATH_MAX], *rel_path;
  struct stat buf;
  alpm_list_t *i;
- size_t len, is_dir;
+ size_t len;
  unsigned int found = 0;
+ int is_dir;
 
  if((filename = strdup(t->data)) == NULL) {
  goto targcleanup;
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v3 2/2] src/pacman/query.c: do not exit -Qo with error if file does not exist

Ivy Foster-2
From: Ivy Foster <[hidden email]>

Query operations act on the local db, not the filesystem. Also, a
valid use case for -Qo is to discover what package owns a deleted file
so it can be reinstalled.

Closes FS#55856.

Signed-off-by: Ivy Foster <[hidden email]>
---
My apologies for missing the earlier patch's problems with missing
dirs. Frankly, I don't know how I missed it. This one really,
honest-to-goodness *does* find missing dirs, as long the user passes
the dirname with a slash at the end, as well as missing files. Enjoy!

 src/pacman/query.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/pacman/query.c b/src/pacman/query.c
index 91ca78a7..39f8dabf 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -160,7 +160,7 @@ static int query_fileowner(alpm_list_t *targets)
  alpm_list_t *i;
  size_t len;
  unsigned int found = 0;
- int is_dir;
+ int is_dir = 0, is_missing = 0;
 
  if((filename = strdup(t->data)) == NULL) {
  goto targcleanup;
@@ -175,27 +175,21 @@ static int query_fileowner(alpm_list_t *targets)
  len = strlen(filename) - 1;
  while(len > 0 && filename[len] == '/') {
  filename[len--] = '\0';
+ /* If a non-dir file exists, S_ISDIR will correct this later. */
+ is_dir = 1;
  }
 
  if(lstat(filename, &buf) == -1) {
+ is_missing = 1;
  /* if it is not a path but a program name, then check in PATH */
- if(strchr(filename, '/') == NULL) {
- if(search_path(&filename, &buf) == -1) {
- pm_printf(ALPM_LOG_ERROR, _("failed to find '%s' in PATH: %s\n"),
- filename, strerror(errno));
- goto targcleanup;
- }
- } else {
- pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
- filename, strerror(errno));
- goto targcleanup;
+ if ((strchr(filename, '/') == NULL) && (search_path(&filename, &buf) == 0)) {
+ is_missing = 0;
  }
  }
 
  if(!lrealpath(filename, rpath)) {
- pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"),
- filename, strerror(errno));
- goto targcleanup;
+ /* Can't canonicalize path, try to proceed anyway */
+ strncpy(rpath, filename, PATH_MAX);
  }
 
  if(strncmp(rpath, root, rootlen) != 0) {
@@ -206,7 +200,7 @@ static int query_fileowner(alpm_list_t *targets)
 
  rel_path = rpath + rootlen;
 
- if((is_dir = S_ISDIR(buf.st_mode))) {
+ if((is_missing && is_dir) || (is_dir = S_ISDIR(buf.st_mode))) {
  size_t rlen = strlen(rpath);
  if(rlen + 2 >= PATH_MAX) {
  pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath);
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v3 2/2] src/pacman/query.c: do not exit -Qo with error if file does not exist

Andrew Gregory
On 01/24/18 at 07:20pm, [hidden email] wrote:

> From: Ivy Foster <[hidden email]>
>
> Query operations act on the local db, not the filesystem. Also, a
> valid use case for -Qo is to discover what package owns a deleted file
> so it can be reinstalled.
>
> Closes FS#55856.
>
> Signed-off-by: Ivy Foster <[hidden email]>
> ---
> My apologies for missing the earlier patch's problems with missing
> dirs. Frankly, I don't know how I missed it. This one really,
> honest-to-goodness *does* find missing dirs, as long the user passes
> the dirname with a slash at the end, as well as missing files. Enjoy!
>
>  src/pacman/query.c | 24 +++++++++---------------
>  1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/src/pacman/query.c b/src/pacman/query.c
> index 91ca78a7..39f8dabf 100644
> --- a/src/pacman/query.c
> +++ b/src/pacman/query.c

...

> @@ -206,7 +200,7 @@ static int query_fileowner(alpm_list_t *targets)
>  
>   rel_path = rpath + rootlen;
>  
> - if((is_dir = S_ISDIR(buf.st_mode))) {
> + if((is_missing && is_dir) || (is_dir = S_ISDIR(buf.st_mode))) {
>   size_t rlen = strlen(rpath);
>   if(rlen + 2 >= PATH_MAX) {
>   pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath);

The call to S_ISDIR needs to be moved up with the stat call(s) in
a branch where we know the stat succeeded.  If the file does not exist
the calls to stat will fail and buf.st_mode will be uninitialized.

apg
Reply | Threaded
Open this post in threaded view
|

[PATCH v4 1/2] pacman/query.c: in query_fileowner, make is_dir an int

Ivy Foster-2
From: Ivy Foster <[hidden email]>

S_ISDIR is int and "returns non-zero" if the file is a directory.

Signed-off-by: Ivy Foster <[hidden email]>
---
 src/pacman/query.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/pacman/query.c b/src/pacman/query.c
index 4a37a338..91ca78a7 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -158,8 +158,9 @@ static int query_fileowner(alpm_list_t *targets)
  char rpath[PATH_MAX], *rel_path;
  struct stat buf;
  alpm_list_t *i;
- size_t len, is_dir;
+ size_t len;
  unsigned int found = 0;
+ int is_dir;
 
  if((filename = strdup(t->data)) == NULL) {
  goto targcleanup;
--
2.16.1
Reply | Threaded
Open this post in threaded view
|

[PATCH v4 2/2] src/pacman/query.c: do not exit -Qo with error if file does not exist

Ivy Foster-2
From: Ivy Foster <[hidden email]>

Query operations act on the local db, not the filesystem. Also, a
valid use case for -Qo is to discover what package owns a deleted file
so it can be reinstalled.

Closes FS#55856.

Signed-off-by: Ivy Foster <[hidden email]>
---
This version checks to make sure that S_ISDIR is only called on extant
files: if lstat fails, above, is_missing is set. If search_path finds
a file of the given name in $PATH, it runs lstat and properly sets
buf, so we're covered either way.

 src/pacman/query.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/src/pacman/query.c b/src/pacman/query.c
index 91ca78a7..5c4e0314 100644
--- a/src/pacman/query.c
+++ b/src/pacman/query.c
@@ -160,7 +160,7 @@ static int query_fileowner(alpm_list_t *targets)
  alpm_list_t *i;
  size_t len;
  unsigned int found = 0;
- int is_dir;
+ int is_dir = 0, is_missing = 0;
 
  if((filename = strdup(t->data)) == NULL) {
  goto targcleanup;
@@ -175,27 +175,21 @@ static int query_fileowner(alpm_list_t *targets)
  len = strlen(filename) - 1;
  while(len > 0 && filename[len] == '/') {
  filename[len--] = '\0';
+ /* If a non-dir file exists, S_ISDIR will correct this later. */
+ is_dir = 1;
  }
 
  if(lstat(filename, &buf) == -1) {
+ is_missing = 1;
  /* if it is not a path but a program name, then check in PATH */
- if(strchr(filename, '/') == NULL) {
- if(search_path(&filename, &buf) == -1) {
- pm_printf(ALPM_LOG_ERROR, _("failed to find '%s' in PATH: %s\n"),
- filename, strerror(errno));
- goto targcleanup;
- }
- } else {
- pm_printf(ALPM_LOG_ERROR, _("failed to read file '%s': %s\n"),
- filename, strerror(errno));
- goto targcleanup;
+ if ((strchr(filename, '/') == NULL) && (search_path(&filename, &buf) == 0)) {
+ is_missing = 0;
  }
  }
 
  if(!lrealpath(filename, rpath)) {
- pm_printf(ALPM_LOG_ERROR, _("cannot determine real path for '%s': %s\n"),
- filename, strerror(errno));
- goto targcleanup;
+ /* Can't canonicalize path, try to proceed anyway */
+ strncpy(rpath, filename, PATH_MAX);
  }
 
  if(strncmp(rpath, root, rootlen) != 0) {
@@ -206,7 +200,7 @@ static int query_fileowner(alpm_list_t *targets)
 
  rel_path = rpath + rootlen;
 
- if((is_dir = S_ISDIR(buf.st_mode))) {
+ if((is_missing && is_dir) || (!is_missing && (is_dir = S_ISDIR(buf.st_mode)))) {
  size_t rlen = strlen(rpath);
  if(rlen + 2 >= PATH_MAX) {
  pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath);
--
2.16.1