[pacman-dev] [PATCH] Use archive_entry_set_perm instead of archive_entry_set_mode

[Xavier]

On Sun, Jan 18, 2009 at 8:23 PM, Nagy Gabor <ngaba at bibl.u-szeged.hu> wrote:
> >From 2b1a58bda8318b7d0c70a4cfc161a82a156b199b Mon Sep 17 00:00:00 2001
> From: Nagy Gabor <ngaba at bibl.u-szeged.hu>
> Date: Sun, 18 Jan 2009 20:03:56 +0100
> Subject: [PATCH] Use archive_entry_set_perm instead of archive_entry_set_mode
>
> This patch fixes FS#12148 ('unstable' regular file).
> I also changed the other archive_entry_set_mode usage in add.c to
> archive_entry_set_perm.
>
> Since I cannot find any relevant info in libarchive manual, I quote
> Tim Kientzle (the author of libarchive) here, and I say thank you for
> his help.
>
> *** Tim Kientzle wrote *************************************
>
> This is the problem in libalpm/util.c:
>
> 323    if(S_ISREG(st->st_mode)) {
> 324            archive_entry_set_mode(entry, 0644);
> 325    } else if(S_ISDIR(st->st_mode)) {
> 326            archive_entry_set_mode(entry, 0755);
> 327    }
>
> Your example unstable.db.tar.gz is not empty.  It has
> one entry in it, called "./".  That entry is marked
> as a directory.  But, when you call archive_entry_set_mode(),
> you are changing the file type!  archive_read_extract()
> then creates the file /var/unstable as you requested.
> (archive_read_extract() will replace an empty directory
> with a file.)
>
> You should either set the mode value correctly:
>
> 323    if(S_ISREG(st->st_mode)) {
> 324            archive_entry_set_mode(entry, IFREG | 0644);
> 325    } else if(S_ISDIR(st->st_mode)) {
> 326            archive_entry_set_mode(entry, IFDIR | 0755);
> 327    }
>
> Or use archive_entry_set_perm(), which does not change
> the file type:
>
> 323    if(S_ISREG(st->st_mode)) {
> 324            archive_entry_set_perm(entry, 0644);
> 325    } else if(S_ISDIR(st->st_mode)) {
> 326            archive_entry_set_perm(entry, 0755);
> 327    }
>
> ************************************************************
>
> Signed-off-by: Nagy Gabor <ngaba at bibl.u-szeged.hu>
> ---
>  lib/libalpm/add.c  |    4 ++--
>  lib/libalpm/util.c |    4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> index 23659c9..3c0074f 100644
> --- a/lib/libalpm/add.c
> +++ b/lib/libalpm/add.c
> @@ -292,12 +292,12 @@ static int extract_single_file(struct archive *archive,
>                /* the install script goes inside the db */
>                snprintf(filename, PATH_MAX, "%s%s-%s/install", db->path,
>                                newpkg->name, newpkg->version);
> -               archive_entry_set_mode(entry, 0644);
> +               archive_entry_set_perm(entry, 0644);
>        } else if(strcmp(entryname, ".CHANGELOG") == 0) {
>                /* the changelog goes inside the db */
>                snprintf(filename, PATH_MAX, "%s%s-%s/changelog", db->path,
>                                newpkg->name, newpkg->version);
> -               archive_entry_set_mode(entry, 0644);
> +               archive_entry_set_perm(entry, 0644);
>        } else if(*entryname == '.') {
>                /* for now, ignore all files starting with '.' that haven't
>                 * already been handled (for future possibilities) */
> diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> index 5e6ca0c..1bec634 100644
> --- a/lib/libalpm/util.c
> +++ b/lib/libalpm/util.c
> @@ -321,9 +321,9 @@ int _alpm_unpack(const char *archive, const char *prefix, const char *fn)
>                entryname = archive_entry_pathname(entry);
>
>                if(S_ISREG(st->st_mode)) {
> -                       archive_entry_set_mode(entry, 0644);
> +                       archive_entry_set_perm(entry, 0644);
>                } else if(S_ISDIR(st->st_mode)) {
> -                       archive_entry_set_mode(entry, 0755);
> +                       archive_entry_set_perm(entry, 0755);
>                }
>
>                /* If a specific file was requested skip entries that don't match. */
> --
> 1.6.1
>

Thank you for contacting Tim about this issue and thanks Tim for his
help, it is always appreciated :)
I made another change to alpm_unpack which is good in my opinion, but
it only worked around this problem and hid it.
http://bugs.archlinux.org/task/12148#comment37568