[Aptitude-svn-commit] r4314 - in branches/aptitude-0.3/aptitude: .
src
Daniel Burrows
dburrows at costa.debian.org
Tue Sep 27 20:09:17 UTC 2005
Author: dburrows
Date: Tue Sep 27 20:09:14 2005
New Revision: 4314
Modified:
branches/aptitude-0.3/aptitude/ChangeLog
branches/aptitude-0.3/aptitude/src/ui.cc
Log:
Use the temp:: stuff for the temporary communication files when su-ing to root.
Modified: branches/aptitude-0.3/aptitude/ChangeLog
==============================================================================
--- branches/aptitude-0.3/aptitude/ChangeLog (original)
+++ branches/aptitude-0.3/aptitude/ChangeLog Tue Sep 27 20:09:14 2005
@@ -1,5 +1,10 @@
2005-09-27 Daniel Burrows <dburrows at debian.org>
+ * src/ui.cc:
+
+ When su-ing to root, use temp::* instead of making the needed
+ temporary names manually.
+
* src/generic/apt/pkg_changelog.cc, src/generic/apt/pkg_changeloh.h:
Use the temp::* classes to manage the changelog instead of
Modified: branches/aptitude-0.3/aptitude/src/ui.cc
==============================================================================
--- branches/aptitude-0.3/aptitude/src/ui.cc (original)
+++ branches/aptitude-0.3/aptitude/src/ui.cc Tue Sep 27 20:09:14 2005
@@ -87,6 +87,7 @@
#include <generic/problemresolver/exceptions.h>
#include <generic/problemresolver/solution.h>
+#include <generic/util/temp.h>
#include <generic/util/util.h>
#include "dep_item.h"
@@ -357,10 +358,14 @@
// Because tagfiles don't work on FIFOs, and su closes all open fds,
// this is a lot hairier than it should be.
//
-// This writes the current status to a file in ~/.aptitude/.tmp, then
-// loads it in a su'd instance. A FIFO is used to make the reader
-// block until the writer is done writing. Not foolproof but the user
-// would have to go to a lot of pointless trouble to screw it up.
+// This writes the current status to a file in a designated temporary
+// directory, then loads it in a su'd instance. A FIFO is used to
+// make the reader block until the writer is done writing. Not
+// foolproof but the user would have to go to a lot of pointless
+// trouble to screw it up.
+//
+// Note that the deletion of the temporary files is safe, since this
+// routine blocks until the sub-process exits.
static void do_su_to_root(string args)
{
@@ -370,47 +375,14 @@
return;
}
- // Copied from pkg_changelog.cc. This should be its own routine?
- string fifoname="aptitudeXXXXXX", statusfile="aptitudeXXXXXX";
+ temp::dir tempdir("aptitude");
+ temp::name statusname(tempdir, "pkgstates");
+ temp::name fifoname(tempdir, "control");
- string dotdir(get_homedir());
- if(!dotdir.empty())
- {
- dotdir+="/.aptitude";
- if(mkdir(dotdir.c_str(), 0700)<0 && errno!=EEXIST)
- {
- _error->Errno("mkdir", "%s", dotdir.c_str());
- return;
- }
- string tmpdotdir(dotdir+"/.tmp");
- if(mkdir(tmpdotdir.c_str(), 0700)<0 && errno!=EEXIST)
- {
- _error->Errno("mkdir", "%s", tmpdotdir.c_str());
- return;
- }
- fifoname=tmpdotdir+"/"+fifoname;
- statusfile=tmpdotdir+"/"+statusfile;
- }
- else
+ if(mkfifo(fifoname.get_name().c_str(), 0600) != 0)
{
- _error->Warning("Can't get value of $HOME, using TMPDIR (insecure)");
- if(getenv("TMPDIR"))
- {
- fifoname=getenv("TMPDIR")+("/"+fifoname);
- statusfile=getenv("TMPDIR")+("/"+statusfile);
- }
- else
- {
- fifoname="/tmp/"+fifoname;
- statusfile="/tmp/"+statusfile;
- }
- }
- fifoname=mktemp((char *) fifoname.c_str());
- statusfile=mktemp((char *) statusfile.c_str());
-
- if(mkfifo(fifoname.c_str(), 0600)==-1)
- {
- _error->Error("Couldn't create temporary FIFO");
+ _error->Errno("do_su_to_root",
+ "Couldn't create temporary FIFO");
return;
}
@@ -422,7 +394,7 @@
{
// Read one byte from the FIFO for synchronization
char tmp;
- int fd=open(fifoname.c_str(), O_RDONLY);
+ int fd = open(fifoname.get_name().c_str(), O_RDONLY);
read(fd, &tmp, 1); // Will block until the other process writes.
close(fd);
@@ -430,12 +402,12 @@
// since the user has to authenticate themselves as root (ie, if
// they're going to do something evil that way they'd have su'd
// directly)
+ //
+ // What happens if tmpdir has spaces in it? Can I get more
+ // control over how the su-to-root function is called?
char cmdbuf[512];
snprintf(cmdbuf, 512, "%s -S %s %s",
- argv0, statusfile.c_str(), args.c_str());
-
- // I think I shall assume that su is in a sane place and behaves
- // sanely.
+ argv0, statusname.get_name().c_str(), args.c_str());
execl("/bin/su", "/bin/su", "-c", cmdbuf, NULL);
}
else
@@ -444,14 +416,14 @@
OpProgress foo; // Need a generic non-outputting progress bar
// Save the selection list.
- (*apt_cache_file)->save_selection_list(foo, statusfile.c_str());
+ (*apt_cache_file)->save_selection_list(foo, statusname.get_name().c_str());
// Shut curses down.
vscreen_suspend();
// Ok, wake the other process up.
char tmp=0;
- int fd=open(fifoname.c_str(), O_WRONLY);
+ int fd=open(fifoname.get_name().c_str(), O_WRONLY);
write(fd, &tmp, 1);
close(fd);
@@ -459,8 +431,6 @@
while(waitpid(pid, &status, 0)!=pid)
;
- unlink(fifoname.c_str());
-
if(!WIFEXITED(status) || WEXITSTATUS(status))
{
_error->Error("%s",
@@ -470,14 +440,16 @@
// We have to clear these out or the cache won't reload properly (?)
vs_progress_ref p = gen_progress_bar();
- apt_reload_cache(p.unsafe_get_ref(), true, statusfile.c_str());
+ apt_reload_cache(p.unsafe_get_ref(), true, statusname.get_name().c_str());
p->destroy();
-
- unlink(statusfile.c_str());
}
else
{
- unlink(statusfile.c_str());
+ // Clear out our references to these objects so they get
+ // removed.
+ tempdir = temp::dir();
+ statusname = temp::name();
+ fifoname = temp::name();
exit(0);
}
More information about the Aptitude-svn-commit
mailing list