FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 04-29-2008, 03:34 AM
Dan McGee
 
Default Rework extract_single_file() temp file creation

We were a bit juryrigged using one call to mkstemp() before rather than
extracting the new files side-by-side and doing our comparisons there. We
were also facing some permissions issues. Instead, make our life easier by
extracting all temp files to a '.paccheck' extension, doing our md5
comparisons, and then taking the correct actions.

Still to be done here- a cleanup of the use of PATH_MAX which should not be
necessary if we use dynamic allocation on the heap.

Signed-off-by: Dan McGee <dan@archlinux.org>
---
lib/libalpm/add.c | 64 +++++++++++++++++++++++++---------------------
pactest/tests/mode003.py | 20 ++++++++++++++
2 files changed, 55 insertions(+), 29 deletions(-)
create mode 100644 pactest/tests/mode003.py

diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
index 6795d52..f759e7d 100644
--- a/lib/libalpm/add.c
+++ b/lib/libalpm/add.c
@@ -434,20 +434,14 @@ static int extract_single_file(struct archive *archive,
}

if(needbackup) {
- char *tempfile;
+ char checkfile[PATH_MAX];
char *hash_local = NULL, *hash_pkg = NULL;
- int fd;
int ret;

- /* extract the package's version to a temporary file and checksum it */
- STRDUP(tempfile, "/tmp/alpm_XXXXXX", RET_ERR(PM_ERR_MEMORY, -1));
- fd = mkstemp(tempfile);
- if(fd == -1) {
- RET_ERR(PM_ERR_SYSTEM, -1);
- }
+ snprintf(checkfile, PATH_MAX, "%s.paccheck", filename);
+ archive_entry_set_pathname(entry, checkfile);

- ret = archive_read_data_into_fd(archive, fd);
- close(fd);
+ ret = archive_read_extract(archive, entry, archive_flags);
if(ret == ARCHIVE_WARN) {
/* operation succeeded but a non-critical error was encountered */
_alpm_log(PM_LOG_DEBUG, "warning extracting %s (%s)
",
@@ -457,14 +451,12 @@ static int extract_single_file(struct archive *archive,
entryname, archive_error_string(archive));
alpm_logaction("error: could not extract %s (%s)
",
entryname, archive_error_string(archive));
- unlink(tempfile);
- FREE(tempfile);
FREE(hash_orig);
return(1);
}

hash_local = alpm_get_md5sum(filename);
- hash_pkg = alpm_get_md5sum(tempfile);
+ hash_pkg = alpm_get_md5sum(checkfile);

/* append the new md5 hash to it's respective entry
* in newpkg's backup (it will be the new orginal) */
@@ -500,14 +492,18 @@ static int extract_single_file(struct archive *archive,

/* move the existing file to the "pacorig" */
if(rename(filename, newpath)) {
- _alpm_log(PM_LOG_ERROR, _("could not rename %s (%s)
"), filename, strerror(errno));
- alpm_logaction("error: could not rename %s (%s)
", filename, strerror(errno));
+ _alpm_log(PM_LOG_ERROR, _("could not rename %s to %s (%s)
"),
+ filename, newpath, strerror(errno));
+ alpm_logaction("error: could not rename %s to %s (%s)
",
+ filename, newpath, strerror(errno));
errors++;
} else {
- /* copy the tempfile we extracted to the real path */
- if(_alpm_copyfile(tempfile, filename)) {
- _alpm_log(PM_LOG_ERROR, _("could not copy tempfile to %s (%s)
"), filename, strerror(errno));
- alpm_logaction("error: could not copy tempfile to %s (%s)
", filename, strerror(errno));
+ /* rename the file we extracted to the real name */
+ if(rename(checkfile, filename)) {
+ _alpm_log(PM_LOG_ERROR, _("could not rename %s to %s (%s)
"),
+ checkfile, filename, strerror(errno));
+ alpm_logaction("error: could not rename %s to %s (%s)
",
+ checkfile, filename, strerror(errno));
errors++;
} else {
_alpm_log(PM_LOG_WARNING, _("%s saved as %s
"), filename, newpath);
@@ -524,35 +520,45 @@ static int extract_single_file(struct archive *archive,
_alpm_log(PM_LOG_DEBUG, "action: installing new file: %s
",
entryname);

- if(_alpm_copyfile(tempfile, filename)) {
- _alpm_log(PM_LOG_ERROR, _("could not copy tempfile to %s (%s)
"), filename, strerror(errno));
+ if(rename(checkfile, filename)) {
+ _alpm_log(PM_LOG_ERROR, _("could not rename %s to %s (%s)
"),
+ checkfile, filename, strerror(errno));
+ alpm_logaction("error: could not rename %s to %s (%s)
",
+ checkfile, filename, strerror(errno));
errors++;
}
} else {
/* there's no sense in installing the same file twice, install
* ONLY is the original and package hashes differ */
_alpm_log(PM_LOG_DEBUG, "action: leaving existing file in place
");
+ unlink(checkfile);
}
} else if(strcmp(hash_orig, hash_pkg) == 0) {
/* originally installed file and new file are the same - this
* implies the case above failed - i.e. the file was changed by a
* user */
_alpm_log(PM_LOG_DEBUG, "action: leaving existing file in place
");
+ unlink(checkfile);
} else if(strcmp(hash_local, hash_pkg) == 0) {
/* this would be magical. The above two cases failed, but the
* user changes just so happened to make the new file exactly the
* same as the one in the package... skip it */
_alpm_log(PM_LOG_DEBUG, "action: leaving existing file in place
");
+ unlink(checkfile);
} else {
char newpath[PATH_MAX];
_alpm_log(PM_LOG_DEBUG, "action: keeping current file and installing new one with .pacnew ending
");
snprintf(newpath, PATH_MAX, "%s.pacnew", filename);
- if(_alpm_copyfile(tempfile, newpath)) {
- _alpm_log(PM_LOG_ERROR, _("could not install %s as %s: %s
"), filename, newpath, strerror(errno));
- alpm_logaction("error: could not install %s as %s: %s
", filename, newpath, strerror(errno));
+ if(rename(checkfile, newpath)) {
+ _alpm_log(PM_LOG_ERROR, _("could not install %s as %s (%s)
"),
+ filename, newpath, strerror(errno));
+ alpm_logaction("error: could not install %s as %s (%s)
",
+ filename, newpath, strerror(errno));
} else {
- _alpm_log(PM_LOG_WARNING, _("%s installed as %s
"), filename, newpath);
- alpm_logaction("warning: %s installed as %s
", filename, newpath);
+ _alpm_log(PM_LOG_WARNING, _("%s installed as %s
"),
+ filename, newpath);
+ alpm_logaction("warning: %s installed as %s
",
+ filename, newpath);
}
}
}
@@ -560,9 +566,9 @@ static int extract_single_file(struct archive *archive,
FREE(hash_local);
FREE(hash_pkg);
FREE(hash_orig);
- unlink(tempfile);
- FREE(tempfile);
} else {
+ int ret;
+
/* we didn't need a backup */
if(notouch) {
/* change the path to a .pacnew extension */
@@ -583,7 +589,7 @@ static int extract_single_file(struct archive *archive,

archive_entry_set_pathname(entry, filename);

- int ret = archive_read_extract(archive, entry, archive_flags);
+ ret = archive_read_extract(archive, entry, archive_flags);
if(ret == ARCHIVE_WARN) {
/* operation succeeded but a non-critical error was encountered */
_alpm_log(PM_LOG_DEBUG, "warning extracting %s (%s)
",
diff --git a/pactest/tests/mode003.py b/pactest/tests/mode003.py
new file mode 100644
index 0000000..1193a5c
--- /dev/null
+++ b/pactest/tests/mode003.py
@@ -0,0 +1,20 @@
+self.description = "Backup file permissions test (same as orig)"
+
+lp = pmpkg("filesystem")
+lp.files = ["etc/profile|666"]
+lp.backup = ["etc/profile*"]
+self.addpkg2db("local", lp)
+
+p = pmpkg("filesystem", "1.0-2")
+p.files = ["etc/profile|666**"]
+p.backup = ["etc/profile"]
+self.addpkg(p)
+
+self.args = "-U %s" % p.filename()
+
+self.addrule("PACMAN_RETCODE=0")
+self.addrule("!FILE_PACSAVE=etc/profile")
+self.addrule("FILE_PACNEW=etc/profile")
+self.addrule("FILE_EXIST=etc/profile")
+self.addrule("FILE_MODE=etc/profile|666")
+self.addrule("FILE_MODE=etc/profile.pacnew|666")
--
1.5.5.1


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 

Thread Tools




All times are GMT. The time now is 12:28 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org