src: cleanup option handling, add testcases; fixes #587

src/Makefile

@@ -209,7 +209,7 @@ ifeq ($(shell uname),Linux)
 CLANG_FORMAT_FILES += bele.h bele_policy.h
 CLANG_FORMAT_FILES += dt_check.cpp dt_impl.cpp except.cpp except.h
 CLANG_FORMAT_FILES += linker.cpp linker.h packhead.cpp packmast.cpp packmast.h
-CLANG_FORMAT_FILES += main.cpp options.h packer.cpp packer.h
+CLANG_FORMAT_FILES += main.cpp options.cpp options.h packer.cpp packer.h
 CLANG_FORMAT_FILES += p_tos.cpp p_tos.h
 CLANG_FORMAT_FILES += s_djgpp2.cpp s_object.cpp s_vcsa.cpp s_win32.cpp screen.h
 CLANG_FORMAT_FILES += snprintf.cpp

src/conf.h

@@ -738,7 +738,9 @@ bool upx_doctest_check(void);
 
 // main.cpp
 extern const char *progname;
-bool set_exit_code(int ec);
+bool main_set_exit_code(int ec);
+int main_get_options(int argc, char **argv);
+void main_get_envoptions();
 int upx_main(int argc, char *argv[]);
 
 // msg.cpp

src/main.cpp

@@ -35,48 +35,7 @@
 // options
 **************************************************************************/
 
-void options_t::reset() {
+static const char *argv0 = "";
-    options_t *o = this;
-    mem_clear(o, sizeof(*o));
-    o->crp.reset();
-
-    o->cmd = CMD_NONE;
-    o->method = M_NONE;
-    o->level = -1;
-    o->filter = FT_NONE;
-
-    o->backup = -1;
-    o->overlay = -1;
-    o->preserve_mode = true;
-    o->preserve_ownership = true;
-    o->preserve_timestamp = true;
-
-    o->console = CON_FILE;
-#if (ACC_OS_DOS32) && defined(__DJGPP__)
-    o->console = CON_INIT;
-#elif (USE_SCREEN_WIN32)
-    o->console = CON_INIT;
-#elif 1 && defined(__linux__)
-    o->console = CON_INIT;
-#endif
-    o->verbose = 2;
-
-    o->win32_pe.compress_exports = 1;
-    o->win32_pe.compress_icons = 2;
-    o->win32_pe.compress_resources = -1;
-    for (unsigned i = 0; i < TABLESIZE(o->win32_pe.compress_rt); i++)
-        o->win32_pe.compress_rt[i] = -1;
-    o->win32_pe.compress_rt[24] = false; // 24 == RT_MANIFEST
-    o->win32_pe.strip_relocs = -1;
-    o->win32_pe.keep_resource = "";
-}
-
-static options_t global_options;
-options_t *opt = &global_options;
-
-static int done_output_name = 0;
-
-const char *argv0 = "";
 const char *progname = "";
 
 static acc_getopt_t mfx_getopt;
@@ -138,14 +97,18 @@ static bool set_eec(int ec, int *eec) {
     return 0;
 }
 
-bool set_exit_code(int ec) { return set_eec(ec, &exit_code); }
+bool main_set_exit_code(int ec) { return set_eec(ec, &exit_code); }
 
 static void e_exit(int ec) {
-    (void) set_exit_code(ec);
+    if (opt->debug.getopt_throw_instead_of_exit)
+        throw ec;
+    (void) main_set_exit_code(ec);
     do_exit();
 }
 
 static void e_usage(void) {
+    if (opt->debug.getopt_throw_instead_of_exit)
+        throw EXIT_USAGE;
     show_usage();
     e_exit(EXIT_USAGE);
 }
@@ -287,7 +250,7 @@ static bool set_method(int m, int l) {
 
 static void set_output_name(const char *n, bool allow_m) {
 #if 1
-    if (done_output_name > 0) {
+    if (opt->output_name) {
         fprintf(stderr, "%s: option '-o' more than once given\n", argv0);
         e_usage();
     }
@@ -301,7 +264,6 @@ static void set_output_name(const char *n, bool allow_m) {
         e_usage();
     }
     opt->output_name = n;
-    done_output_name++;
 }
 
 /*************************************************************************
@@ -482,13 +444,13 @@ static int do_option(int optc, const char *arg) {
         break;
     case 721:
         opt->method_lzma_seen = true;
-        opt->all_methods_use_lzma = true;
+        opt->all_methods_use_lzma = 1;
         if (!set_method(M_LZMA, -1))
             e_method(M_LZMA, opt->level);
         break;
     case 722:
         opt->method_lzma_seen = false;
-        opt->all_methods_use_lzma = false;
+        opt->all_methods_use_lzma = -1; // explicitly disabled
         if (M_IS_LZMA(opt->method))
             opt->method = -1;
         break;
@@ -518,7 +480,8 @@ static int do_option(int optc, const char *arg) {
         /* fallthrough */
     case 901: // --brute
         opt->all_methods = true;
-        opt->all_methods_use_lzma = true;
+        if (opt->all_methods_use_lzma != -1)
+            opt->all_methods_use_lzma = 1;
         opt->method = -1;
         opt->all_filters = true;
         opt->filter = -1;
@@ -549,17 +512,6 @@ static int do_option(int optc, const char *arg) {
         opt->debug.disable_random_id = true;
         break;
 
-    // mp (meta)
-    case 501:
-        getoptvar(&opt->mp_compress_task, 1, 999999, arg);
-        break;
-    case 502:
-        opt->mp_query_format = true;
-        break;
-    case 503:
-        opt->mp_query_num_tasks = true;
-        break;
-
     // misc
     case 512:
         opt->console = CON_FILE;
@@ -606,7 +558,8 @@ static int do_option(int optc, const char *arg) {
         break;
     case 524: // --all-methods
         opt->all_methods = true;
-        opt->all_methods_use_lzma = true;
+        if (opt->all_methods_use_lzma != -1)
+            opt->all_methods_use_lzma = 1;
         opt->method = -1;
         break;
     case 525: // --exact
@@ -820,7 +773,7 @@ static int do_option(int optc, const char *arg) {
     return 0;
 }
 
-static int get_options(int argc, char **argv) {
+int main_get_options(int argc, char **argv) {
     constexpr int *N = nullptr;
 
     static const struct mfx_option longopts[] = {
@@ -852,9 +805,9 @@ static int get_options(int argc, char **argv) {
         {"quiet", 0, N, 'q'},  // quiet mode
         {"silent", 0, N, 'q'}, // quiet mode
 #if 0
-    // FIXME: to_stdout doesn't work because of console code mess
+        // FIXME: to_stdout doesn't work because of console code mess
-    {"stdout",           0x10, N, 517},     // write output on standard output
+        {"stdout",           0x10, N, 517},     // write output on standard output
-    {"to-stdout",        0x10, N, 517},     // write output on standard output
+        {"to-stdout",        0x10, N, 517},     // write output on standard output
 #endif
         {"verbose", 0, N, 'v'}, // verbose mode
 
@@ -970,11 +923,6 @@ static int get_options(int argc, char **argv) {
         {"8mib-ram", 0x10, N, 673},
         {"8mb-ram", 0x10, N, 673},
 
-        // mp (meta) options
-        {"mp-compress-task", 0x31, N, 501},
-        {"mp-query-format", 0x10, N, 502},
-        {"mp-query-num-tasks", 0x10, N, 503},
-
         {nullptr, 0, nullptr, 0}
     };
 
@@ -994,8 +942,8 @@ static int get_options(int argc, char **argv) {
     return mfx_optind;
 }
 
+void main_get_envoptions() {
 #if defined(OPTIONS_VAR)
-static void get_envoptions(int argc, char **argv) {
     constexpr int *N = nullptr;
 
     /* only some options are allowed in the environment variable */
@@ -1068,7 +1016,7 @@ static void get_envoptions(int argc, char **argv) {
     const char *var;
     int i, optc, longind;
     int targc;
-    char **targv = nullptr;
+    const char **targv = nullptr;
     static const char sep[] = " \t";
     char shortopts[256];
 
@@ -1095,14 +1043,14 @@ static void get_envoptions(int argc, char **argv) {
 
     /* alloc temp argv */
     if (targc > 1)
-        targv = (char **) calloc(targc + 1, sizeof(char *));
+        targv = (const char **) calloc(targc + 1, sizeof(char *));
     if (targv == nullptr) {
         free(env);
         return;
     }
 
     /* fill temp argv */
-    targv[0] = argv[0];
+    targv[0] = argv0;
     for (p = env, targc = 1;;) {
         while (*p && strchr(sep, *p))
             p++;
@@ -1124,7 +1072,7 @@ static void get_envoptions(int argc, char **argv) {
 
     /* handle options */
     prepare_shortopts(shortopts, "123456789", longopts);
-    acc_getopt_init(&mfx_getopt, 1, targc, targv);
+    acc_getopt_init(&mfx_getopt, 1, targc, const_cast<char **>(targv));
     mfx_getopt.progname = progname;
     mfx_getopt.opterr = handle_opterr;
     while ((optc = acc_getopt(&mfx_getopt, shortopts, longopts, &longind)) >= 0) {
@@ -1138,9 +1086,8 @@ static void get_envoptions(int argc, char **argv) {
     /* clean up */
     free(targv);
     free(env);
-    UNUSED(argc);
-}
 #endif /* defined(OPTIONS_VAR) */
+}
 
 static void first_options(int argc, char **argv) {
     int i;
@@ -1181,7 +1128,6 @@ int upx_main(int argc, char *argv[]) {
     }
 
     // Allow serial re-use of upx_main() as a subroutine
-    done_output_name = 0;
     opt->reset();
 
 #if (ACC_OS_CYGWIN || ACC_OS_DOS16 || ACC_OS_DOS32 || ACC_OS_EMX || ACC_OS_TOS || ACC_OS_WIN16 ||  \
@@ -1219,11 +1165,9 @@ int upx_main(int argc, char *argv[]) {
 
     /* get options */
     first_options(argc, argv);
-#if defined(OPTIONS_VAR)
     if (!opt->no_env)
-        get_envoptions(argc, argv);
+        main_get_envoptions();
-#endif
+    i = main_get_options(argc, argv);
-    i = get_options(argc, argv);
     assert(i <= argc);
 
     set_term(nullptr);

src/options.cpp

@@ -0,0 +1,129 @@
+/* options.cpp --
+
+   This file is part of the UPX executable compressor.
+
+   Copyright (C) 1996-2022 Markus Franz Xaver Johannes Oberhumer
+   Copyright (C) 1996-2022 Laszlo Molnar
+   All Rights Reserved.
+
+   UPX and the UCL library are free software; you can redistribute them
+   and/or modify them under the terms of the GNU General Public License as
+   published by the Free Software Foundation; either version 2 of
+   the License, or (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; see the file COPYING.
+   If not, write to the Free Software Foundation, Inc.,
+   59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+   Markus F.X.J. Oberhumer              Laszlo Molnar
+   <markus@oberhumer.com>               <ezerotven+github@gmail.com>
+ */
+
+#include "conf.h"
+
+/*************************************************************************
+// options
+**************************************************************************/
+
+void options_t::reset() {
+    options_t *o = this;
+    mem_clear(o, sizeof(*o));
+    o->crp.reset();
+
+    o->cmd = CMD_NONE;
+    o->method = M_NONE;
+    o->level = -1;
+    o->filter = FT_NONE;
+
+    o->backup = -1;
+    o->overlay = -1;
+    o->preserve_mode = true;
+    o->preserve_ownership = true;
+    o->preserve_timestamp = true;
+
+    o->console = CON_FILE;
+#if (ACC_OS_DOS32) && defined(__DJGPP__)
+    o->console = CON_INIT;
+#elif (USE_SCREEN_WIN32)
+    o->console = CON_INIT;
+#elif 1 && defined(__linux__)
+    o->console = CON_INIT;
+#endif
+    o->verbose = 2;
+
+    o->win32_pe.compress_exports = 1;
+    o->win32_pe.compress_icons = 2;
+    o->win32_pe.compress_resources = -1;
+    for (unsigned i = 0; i < TABLESIZE(o->win32_pe.compress_rt); i++)
+        o->win32_pe.compress_rt[i] = -1;
+    o->win32_pe.compress_rt[24] = false; // 24 == RT_MANIFEST
+    o->win32_pe.strip_relocs = -1;
+    o->win32_pe.keep_resource = "";
+}
+
+static options_t global_options;
+options_t *opt = &global_options;
+
+/*************************************************************************
+//
+**************************************************************************/
+
+template <size_t N>
+static void test_options(const char *(&a)[N]) {
+    (void) main_get_options((int) (N - 1), ACC_UNCONST_CAST(char **, a));
+}
+
+TEST_CASE("getopt") {
+    options_t *saved_opt = opt;
+    options_t local_options;
+    opt = &local_options;
+    opt->reset();
+    opt->debug.getopt_throw_instead_of_exit = true;
+    const char a0[] = "<argv0>";
+
+    SUBCASE("issue 587a") {
+        const char *a[] = {a0, "--brute", "--lzma", nullptr};
+        CHECK(!opt->all_methods);
+        test_options(a);
+        CHECK(opt->all_methods);
+        CHECK(opt->all_methods_use_lzma == 1);
+    }
+    SUBCASE("issue 587b") {
+        const char *a[] = {a0, "--lzma", "--brute", nullptr};
+        CHECK(!opt->all_methods);
+        test_options(a);
+        CHECK(opt->all_methods);
+        CHECK(opt->all_methods_use_lzma == 1);
+    }
+    SUBCASE("issue 587c") {
+        const char *a[] = {a0, "--brute", "--no-lzma", nullptr};
+        CHECK(!opt->all_methods);
+        test_options(a);
+        CHECK(opt->all_methods);
+        CHECK(opt->all_methods_use_lzma == -1);
+    }
+    SUBCASE("issue 587d") {
+        const char *a[] = {a0, "--no-lzma", "--brute", nullptr};
+        CHECK(!opt->all_methods);
+        test_options(a);
+        CHECK(opt->all_methods);
+        CHECK(opt->all_methods_use_lzma == -1);
+    }
+    SUBCASE("issue 587e") {
+        const char *a[] = {a0, "--no-lzma", "--all-methods", nullptr};
+        CHECK(!opt->all_methods);
+        test_options(a);
+        CHECK(opt->all_methods);
+        CHECK(opt->all_methods_use_lzma == -1);
+    }
+
+    opt = saved_opt;
+}
+
+/* vim:set ts=4 sw=4 et: */

src/options.h

@@ -48,11 +48,6 @@ enum {
 struct options_t {
     int cmd;
 
-    // mp (meta) options
-    int mp_compress_task;
-    bool mp_query_format;
-    bool mp_query_num_tasks;
-
     // compression options
     int method;
     bool method_lzma_seen;
@@ -63,7 +58,7 @@ struct options_t {
     int filter; // preferred filter from Packer::getFilters()
     bool ultra_brute;
     bool all_methods; // try all available compression methods ?
-    bool all_methods_use_lzma;
+    int all_methods_use_lzma;
     bool all_filters; // try all available filters ?
     bool no_filter;   // force no filter
     bool prefer_ucl;  // prefer UCL
@@ -90,8 +85,9 @@ struct options_t {
         int debug_level;
         bool disable_random_id; // for Packer::getRandomId()
         const char *dump_stub_loader;
-        char fake_stub_version[4 + 1]; // for internal debugging
+        char fake_stub_version[4 + 1];     // for internal debugging
-        char fake_stub_year[4 + 1];    // for internal debugging
+        char fake_stub_year[4 + 1];        // for internal debugging
+        bool getopt_throw_instead_of_exit; // for doctest
     } debug;
 
     // overlay handling

src/p_exe.cpp

@@ -63,9 +63,10 @@ const int *PackExe::getCompressionMethods(int method, int level) const
 {
     bool small = ih_imagesize <= 256*1024;
     // disable lzma for "--brute" unless explicitly given "--lzma"
-    // WARNING: this side effect persists for later files!
+    // WARNING: this side effect may persists for later files;
-    if (opt->all_methods_use_lzma && !opt->method_lzma_seen)
+    //   but note that class PackMaster creates per-file local options
-        opt->all_methods_use_lzma = false;
+    if (opt->all_methods_use_lzma == 1 && !opt->method_lzma_seen)
+        opt->all_methods_use_lzma = 0;
     return Packer::getDefaultCompressionMethods_8(method, level, small);
 }
 

src/packer.cpp

@@ -1165,7 +1165,7 @@ int Packer::prepareMethods(int *methods, int ph_method, const int *all_methods)
             break;
         if (method == M_SKIP || method == M_ULTRA_BRUTE)
             continue;
-        if (opt->all_methods && !opt->all_methods_use_lzma && M_IS_LZMA(method))
+        if (opt->all_methods && opt->all_methods_use_lzma != 1 && M_IS_LZMA(method))
             continue;
         // use this method
         assert(Packer::isValidCompressionMethod(method));

src/work.cpp

@@ -271,40 +271,40 @@ int do_files(int i, int argc, char *argv[]) {
             unlink_ofile(oname);
             if (opt->verbose >= 1 || (opt->verbose >= 0 && !e.isWarning()))
                 printErr(iname, &e);
-            set_exit_code(e.isWarning() ? EXIT_WARN : EXIT_ERROR);
+            main_set_exit_code(e.isWarning() ? EXIT_WARN : EXIT_ERROR);
             // continue processing more files
         } catch (const Error &e) {
             unlink_ofile(oname);
             printErr(iname, &e);
-            set_exit_code(EXIT_ERROR);
+            main_set_exit_code(EXIT_ERROR);
             return -1;
         } catch (std::bad_alloc *e) {
             unlink_ofile(oname);
             printErr(iname, "out of memory");
             UNUSED(e);
             // delete e;
-            set_exit_code(EXIT_ERROR);
+            main_set_exit_code(EXIT_ERROR);
             return -1;
         } catch (const std::bad_alloc &) {
             unlink_ofile(oname);
             printErr(iname, "out of memory");
-            set_exit_code(EXIT_ERROR);
+            main_set_exit_code(EXIT_ERROR);
             return -1;
         } catch (std::exception *e) {
             unlink_ofile(oname);
             printUnhandledException(iname, e);
             // delete e;
-            set_exit_code(EXIT_ERROR);
+            main_set_exit_code(EXIT_ERROR);
             return -1;
         } catch (const std::exception &e) {
             unlink_ofile(oname);
             printUnhandledException(iname, &e);
-            set_exit_code(EXIT_ERROR);
+            main_set_exit_code(EXIT_ERROR);
             return -1;
         } catch (...) {
             unlink_ofile(oname);
             printUnhandledException(iname, nullptr);
-            set_exit_code(EXIT_ERROR);
+            main_set_exit_code(EXIT_ERROR);
             return -1;
         }
     }