tags 378152 + patch fixed-upstream pending thanks Roger Leigh <[EMAIL PROTECTED]> writes:
> The terminal settings are not being restored on exit (either > on sucess or failure). This looks like a recent regression with > the splitting up of main(). exit() should not be called from > any subclass of schroot_base::main()? The attached patch fixes this by removing all but one exit point in each binary. This ensures the terminal restore is in the exit path under all conditions. -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `- GPG Public Key: 0x25BFB848 Please sign and encrypt your mail.
Index: schroot/schroot-base-main.cc
===================================================================
--- schroot/schroot-base-main.cc (revision 854)
+++ schroot/schroot-base-main.cc (working copy)
@@ -160,11 +160,6 @@
exit(EXIT_FAILURE);
}
- catch (...)
- {
- sbuild::log_error() << _("An unknown exception occured") << endl;
- exit(EXIT_FAILURE);
- }
}
/*
Index: schroot/schroot-main-base.cc
===================================================================
--- schroot/schroot-main-base.cc (revision 854)
+++ schroot/schroot-main-base.cc (working copy)
@@ -38,6 +38,31 @@
using boost::format;
using namespace schroot;
+namespace
+{
+
+ typedef std::pair<main_base::error_code,const char *> emap;
+
+ /**
+ * This is a list of the supported error codes. It's used to
+ * construct the real error codes map.
+ */
+ emap init_errors[] =
+ {
+ emap(main_base::CHROOTS_NOTFOUND, N_("%1%: Chroots not found")),
+ emap(main_base::CHROOT_NOTDEFINED,
+ N_("The specified chroots are not defined in '%1%'")),
+ emap(main_base::CHROOT_NOTFOUND, N_("%1%: Chroot not found"))
+ };
+
+}
+
+template<>
+sbuild::error<main_base::error_code>::map_type
+sbuild::error<main_base::error_code>::error_strings
+(init_errors,
+ init_errors + (sizeof(init_errors) / sizeof(init_errors[0])));
+
main_base::main_base (std::string const& program_name,
std::string const& program_usage,
options_base::ptr& options):
@@ -99,13 +124,19 @@
if (!invalid_chroots.empty())
{
+ std::string invalid_list;
for (sbuild::string_list::const_iterator chroot =
invalid_chroots.begin();
chroot != invalid_chroots.end();
++chroot)
- sbuild::log_error() << format(_("%1%: No such chroot")) % *chroot
- << endl;
- exit(EXIT_FAILURE);
+ {
+ invalid_list += *chroot;
+ if (chroot + 1 != invalid_chroots.end())
+ invalid_list += ", ";
+ }
+ throw error(invalid_list,
+ (invalid_chroots.size() == 1)
+ ? CHROOT_NOTFOUND : CHROOTS_NOTFOUND);
}
ret = this->options->chroots;
}
@@ -135,13 +166,13 @@
if (this->options->action == options_base::ACTION_HELP)
{
action_help(std::cout);
- exit(EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}
if (this->options->action == options_base::ACTION_VERSION)
{
action_version(std::cout);
- exit(EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}
/* Initialise chroot configuration. */
@@ -169,46 +200,31 @@
if (this->options->action == options_base::ACTION_LIST)
{
action_list();
- exit(EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}
/* Get list of chroots to use */
chroots = get_chroot_options();
if (this->chroots.empty())
- {
- sbuild::log_error()
- << format(_("The specified chroots are not defined in '%1%'"))
- % SCHROOT_CONF
- << endl;
- exit (EXIT_FAILURE);
- }
+ throw error(SCHROOT_CONF, CHROOT_NOTDEFINED);
/* Print chroot information for specified chroots. */
if (this->options->action == options_base::ACTION_INFO)
{
action_info();
- exit (EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}
if (this->options->action == options_base::ACTION_LOCATION)
{
action_location();
- exit (EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}
if (this->options->action == options_base::ACTION_CONFIG)
{
action_config();
- exit (EXIT_SUCCESS);
+ return EXIT_SUCCESS;
}
- if (this->options->action == options_base::ACTION_SESSION_BEGIN &&
- this->chroots.size() != 1)
- {
- sbuild::log_error()
- << _("Only one chroot may be specified when beginning a session")
- << endl;
- exit (EXIT_FAILURE);
- }
-
/* Create a session. */
sbuild::session::operation sess_op(sbuild::session::OPERATION_AUTOMATIC);
if (this->options->action == options_base::ACTION_SESSION_BEGIN)
@@ -255,9 +271,9 @@
}
if (this->session)
- exit(this->session->get_child_status());
+ return this->session->get_child_status();
else
- exit(EXIT_FAILURE);
+ return EXIT_FAILURE;
}
/*
Index: schroot/schroot-main-base.h
===================================================================
--- schroot/schroot-main-base.h (revision 846)
+++ schroot/schroot-main-base.h (working copy)
@@ -23,6 +23,8 @@
#include <schroot/schroot-base-main.h>
#include <schroot/schroot-options-base.h>
+#include <sbuild/sbuild-custom-error.h>
+
namespace schroot
{
@@ -34,6 +36,17 @@
class main_base : public schroot_base::main
{
public:
+ /// Error codes.
+ enum error_code
+ {
+ CHROOTS_NOTFOUND, ///< Chroots not found.
+ CHROOT_NOTDEFINED, ///< The specified chroots are not defined.
+ CHROOT_NOTFOUND ///< Chroot not found.
+ };
+
+ /// Exception type.
+ typedef sbuild::custom_error<error_code> error;
+
/**
* The constructor.
*
Index: schroot/schroot-releaselock-main.h
===================================================================
--- schroot/schroot-releaselock-main.h (revision 846)
+++ schroot/schroot-releaselock-main.h (working copy)
@@ -23,6 +23,8 @@
#include <schroot/schroot-base-main.h>
#include <schroot/schroot-releaselock-options.h>
+#include <sbuild/sbuild-custom-error.h>
+
namespace schroot_releaselock
{
@@ -32,6 +34,18 @@
class main : public schroot_base::main
{
public:
+ /// Error codes.
+ enum error_code
+ {
+ DEVICE_NOTBLOCK, ///< File is not a block device.
+ DEVICE_OWNED, ///< Failed to release device lock (lock held by PID).
+ DEVICE_RELEASE, ///< Failed to release device lock.
+ DEVICE_STAT ///< Failed to stat device.
+ };
+
+ /// Exception type.
+ typedef sbuild::custom_error<error_code> error;
+
/**
* The constructor.
*
Index: schroot/schroot-options-base.cc
===================================================================
--- schroot/schroot-options-base.cc (revision 846)
+++ schroot/schroot-options-base.cc (working copy)
@@ -155,7 +155,7 @@
this->load_chroots = true;
this->load_sessions = false;
if (this->chroots.size() != 1 || all_used())
- throw opt::validation_error(_("Only one chroot may be specified when recovering, running or ending a session"));
+ throw opt::validation_error(_("Only one chroot may be specified when beginning a session"));
this->all = this->all_chroots = this->all_sessions = false;
break;
Index: schroot/schroot-releaselock-main.cc
===================================================================
--- schroot/schroot-releaselock-main.cc (revision 854)
+++ schroot/schroot-releaselock-main.cc (working copy)
@@ -39,6 +39,31 @@
using boost::format;
using namespace schroot_releaselock;
+namespace
+{
+
+ typedef std::pair<main::error_code,const char *> emap;
+
+ /**
+ * This is a list of the supported error codes. It's used to
+ * construct the real error codes map.
+ */
+ emap init_errors[] =
+ {
+ emap(main::DEVICE_NOTBLOCK, N_("File is not a block device")),
+ emap(main::DEVICE_OWNED, N_("Failed to release device lock (lock held by PID %4%")),
+ emap(main::DEVICE_RELEASE, N_("Failed to release device lock")),
+ emap(main::DEVICE_STAT, N_("Failed to stat device"))
+};
+
+}
+
+template<>
+sbuild::error<main::error_code>::map_type
+sbuild::error<main::error_code>::error_strings
+(init_errors,
+ init_errors + (sizeof(init_errors) / sizeof(init_errors[0])));
+
main::main (options::ptr& options):
schroot_base::main("schroot-releaselock",
_("[OPTION...] - release a device lock"),
@@ -63,38 +88,16 @@
struct stat statbuf;
if (stat(this->options->device.c_str(), &statbuf) == -1)
- {
- sbuild::log_error()
- << format(_("Failed to stat device '%1%': %2%"))
- % this->options->device % strerror(errno)
- << endl;
- exit (EXIT_FAILURE);
- }
+ throw error(this->options->device, DEVICE_STAT, strerror(errno));
+
if (!S_ISBLK(statbuf.st_mode))
- {
- sbuild::log_error()
- << format(_("'%1%' is not a block device")) % this->options->device
- << endl;
- exit (EXIT_FAILURE);
- }
+ throw error(this->options->device, DEVICE_NOTBLOCK);
pid_t status = dev_unlock(this->options->device.c_str(), this->options->pid);
if (status < 0) // Failure
- {
- sbuild::log_error()
- << format(_("%1%: failed to release device lock"))
- % this->options->device
- << endl;
- exit (EXIT_FAILURE);
- }
+ throw error(this->options->device, DEVICE_RELEASE);
else if (status > 0) // Owned
- {
- sbuild::log_error()
- << format(_("%1%: failed to release device lock owned by pid %2%"))
- % this->options->device % status
- << endl;
- exit (EXIT_FAILURE);
- }
+ throw error(this->options->device, DEVICE_OWNED, status);
}
int
Index: debian/changelog
===================================================================
--- debian/changelog (revision 846)
+++ debian/changelog (working copy)
@@ -10,6 +10,8 @@
single command, separated by spaces. This restores compatibility with
dchroot 0.13 and earlier (Closes: #378028). Thanks to David Liontooth
for reporting this.
+ * Terminal settings are correctly restored under all normal exit
+ conditions (Closes: #378152).
* debian/schroot.docs: Add the contents of debian/docs.
* debian/docs: Remove.
* debian/rules:
@@ -20,7 +22,7 @@
* debian/dchroot-dsa.preinst: New file. Remove
/usr/share/doc/dchroot-dsa.
- -- Roger Leigh <[EMAIL PROTECTED]> Thu, 13 Jul 2006 13:22:20 +0100
+ -- Roger Leigh <[EMAIL PROTECTED]> Fri, 14 Jul 2006 14:43:50 +0100
schroot (0.99.2-1) unstable; urgency=low
Index: sbuild/sbuild-session.cc
===================================================================
--- sbuild/sbuild-session.cc (revision 854)
+++ sbuild/sbuild-session.cc (working copy)
@@ -997,7 +997,7 @@
}
/* This should never be reached */
- exit(EXIT_FAILURE);
+ _exit(EXIT_FAILURE);
}
void
@@ -1087,7 +1087,7 @@
{
log_error() << e.what() << endl;
}
- exit (EXIT_FAILURE);
+ _exit (EXIT_FAILURE);
}
else
{
Index: sbuild/sbuild-run-parts.cc
===================================================================
--- sbuild/sbuild-run-parts.cc (revision 854)
+++ sbuild/sbuild-run-parts.cc (working copy)
@@ -179,7 +179,7 @@
exec(this->directory + '/' + file, command, env);
error e(file, EXEC, strerror(errno));
log_error() << e.what() << std::endl;
- exit(EXIT_FAILURE);
+ _exit(EXIT_FAILURE);
}
else
{
Index: ChangeLog
===================================================================
--- ChangeLog (revision 855)
+++ ChangeLog (working copy)
@@ -1,5 +1,30 @@
2006-07-14 Roger Leigh <[EMAIL PROTECTED]>
+ * debian/changelog: Close #378152.
+
+ * Having a single exit point now means terminal settings are
+ always restored correctly.
+
+ * sbuild/sbuild-run-parts.cc: Use _exit rather than exit when
+ terminating a child process when execve has failed.
+
+ * sbuild/sbuild-session.cc: Use _exit rather than exit when
+ terminating a child process when execve has failed.
+
+ * schroot/schroot-releaselock-main.(cc|h): Add error_code enum and
+ error typedef for sbuild::custom_error. Throw error in place of
+ exiting with EXIT_FAILURE.
+
+ * schroot/schroot-main-base.(cc|h): Add error_code enum and error
+ typedef for sbuild::custom_error. Throw error in place of exiting
+ with EXIT_FAILURE. Don't ever exit successfully; return a success
+ status.
+
+ * schroot/schroot-base-main.cc (run): Don't catch "..."; it's
+ handled by the main() stubs.
+
+2006-07-14 Roger Leigh <[EMAIL PROTECTED]>
+
* sbuild/sbuild-chroot-directory.cc (setup_env): Remove.
CHROOT_LOCATION is already set in the parent class.
pgp2H67pglsyk.pgp
Description: PGP signature
