From 2ad6aa134d9e8e755c8c738d0b51d72b0851c212 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 24 Jun 2019 11:25:05 +0200 Subject: Constrain access to options to build system driver main() only --- build2/b.cxx | 114 +++++++++++++++++++++++-------------- build2/bin/init.cxx | 4 +- build2/cc/module.cxx | 4 +- build2/context.cxx | 11 ++-- build2/depdb.ixx | 6 +- build2/diagnostics.cxx | 30 ++++++++-- build2/diagnostics.hxx | 5 +- build2/function.test.cxx | 5 +- build2/operation.cxx | 2 +- build2/test/script/parser.test.cxx | 5 +- build2/utility.cxx | 28 ++++----- build2/utility.hxx | 56 +++++++++++++----- 12 files changed, 172 insertions(+), 98 deletions(-) (limited to 'build2') diff --git a/build2/b.cxx b/build2/b.cxx index a64c5e2..0eb007c 100644 --- a/build2/b.cxx +++ b/build2/b.cxx @@ -67,6 +67,8 @@ using namespace std; namespace build2 { + static options ops; + int main (int argc, char* argv[]); @@ -180,13 +182,13 @@ main (int argc, char* argv[]) } #endif -// A data race happens in the libstdc++ (as of GCC 7.2) implementation of the -// ctype::narrow() function (bug #77704). The issue is easily triggered -// by the testscript runner that indirectly (via regex) uses ctype facet -// of the global locale (and can potentially be triggered by other locale- -// aware code). We work around this by pre-initializing the global locale -// facet internal cache. -// + // A data race happens in the libstdc++ (as of GCC 7.2) implementation of + // the ctype::narrow() function (bug #77704). The issue is easily + // triggered by the testscript runner that indirectly (via regex) uses + // ctype facet of the global locale (and can potentially be triggered + // by other locale- aware code). We work around this by pre-initializing the + // global locale facet internal cache. + // #ifdef __GLIBCXX__ { const ctype& ct (use_facet> (locale ())); @@ -196,26 +198,24 @@ main (int argc, char* argv[]) } #endif - try - { - // On POSIX ignore SIGPIPE which is signaled to a pipe-writing process if - // the pipe reading end is closed. Note that by default this signal - // terminates a process. Also note that there is no way to disable this - // behavior on a file descriptor basis or for the write() function call. - // - // On Windows disable displaying error reporting dialog box for the current - // and child processes unless we run serially. This way we avoid multiple - // dialog boxes to potentially pop up. - // + // On POSIX ignore SIGPIPE which is signaled to a pipe-writing process if + // the pipe reading end is closed. Note that by default this signal + // terminates a process. Also note that there is no way to disable this + // behavior on a file descriptor basis or for the write() function call. + // #ifndef _WIN32 - if (signal (SIGPIPE, SIG_IGN) == SIG_ERR) - fail << "unable to ignore broken pipe (SIGPIPE) signal: " - << system_error (errno, generic_category ()); // Sanitize. + if (signal (SIGPIPE, SIG_IGN) == SIG_ERR) + fail << "unable to ignore broken pipe (SIGPIPE) signal: " + << system_error (errno, generic_category ()); // Sanitize. #endif - // Parse the command line. We want to be able to specify options, vars, - // and buildspecs in any order (it is really handy to just add -v at the - // end of the command line). + // Parse the command line. + // + try + { + // We want to be able to specify options, vars, and buildspecs in any + // order (it is really handy to just add -v at the end of the command + // line). // strings cmd_vars; string args; @@ -336,29 +336,32 @@ main (int argc, char* argv[]) else args += ')'; } + + // Validate options. + // + if (ops.progress () && ops.no_progress ()) + fail << "both --progress and --no-progress specified"; + + if (ops.mtime_check () && ops.no_mtime_check ()) + fail << "both --mtime-check and --no-mtime-check specified"; } catch (const cl::exception& e) { fail << e; } - // Validate options. + // Initialize the diagnostics state. // - if (ops.progress () && ops.no_progress ()) - fail << "both --progress and --no-progress specified"; - - if (ops.mtime_check () && ops.no_mtime_check ()) - fail << "both --mtime-check and --no-mtime-check specified"; - - // Global initializations. - // - stderr_term = fdterm (stderr_fd ()); - init (argv[0], - ops.verbose_specified () - ? ops.verbose () - : ops.V () ? 3 : ops.v () ? 2 : ops.quiet () ? 0 : 1); - - // Version. + init_diag ((ops.verbose_specified () + ? ops.verbose () + : ops.V () ? 3 : ops.v () ? 2 : ops.quiet () ? 0 : 1), + (ops.progress () ? optional (true) : + ops.no_progress () ? optional (false) : nullopt), + ops.no_line (), + ops.no_column (), + fdterm (stderr_fd ())); + + // Handle --version. // if (ops.version ()) { @@ -370,7 +373,7 @@ main (int argc, char* argv[]) return 0; } - // Help. + // Handle --help. // if (ops.help ()) { @@ -396,8 +399,33 @@ main (int argc, char* argv[]) } } + // Initialize time conversion data that is used by localtime_r(). + // +#ifndef _WIN32 + tzset (); +#else + _tzset (); +#endif + + // Initialize the global state. + // + init (argv[0], + !ops.serial_stop (), ops.dry_run (), + (ops.mtime_check () ? optional (true) : + ops.no_mtime_check () ? optional (false) : nullopt), + (ops.config_sub_specified () + ? optional (ops.config_sub ()) + : nullopt), + (ops.config_guess_specified () + ? optional (ops.config_guess ()) + : nullopt)); + #ifdef _WIN32 - if (!ops.serial_stop ()) + // On Windows disable displaying error reporting dialog box for the + // current and child processes unless we are in the stop mode. Failed that + // we may have multiple dialog boxes popping up. + // + if (keep_going) SetErrorMode (SetErrorMode (0) | // Returns the current mode. SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX); #endif @@ -450,8 +478,6 @@ main (int argc, char* argv[]) #endif } - keep_going = !ops.serial_stop (); - // Start up the scheduler and allocate lock shards. // size_t jobs (0); diff --git a/build2/bin/init.cxx b/build2/bin/init.cxx index 3a91646..cf47fbe 100644 --- a/build2/bin/init.cxx +++ b/build2/bin/init.cxx @@ -296,10 +296,10 @@ namespace build2 // Did the user ask us to use config.sub? If this is a hinted value, // then we assume it has already been passed through config.sub. // - if (!hint && ops.config_sub_specified ()) + if (!hint && config_sub) { s = run (3, - ops.config_sub (), + *config_sub, s.c_str (), [] (string& l, bool) {return move (l);}); l5 ([&]{trace << "config.sub target: '" << s << "'";}); diff --git a/build2/cc/module.cxx b/build2/cc/module.cxx index 00c5aa9..7ea790b 100644 --- a/build2/cc/module.cxx +++ b/build2/cc/module.cxx @@ -138,10 +138,10 @@ namespace build2 { string ct; - if (ops.config_sub_specified ()) + if (config_sub) { ct = run (3, - ops.config_sub (), + *config_sub, ci.target.c_str (), [] (string& l, bool) {return move (l);}); l5 ([&]{trace << "config.sub target: '" << ct << "'";}); diff --git a/build2/context.cxx b/build2/context.cxx index a4ff001..e71201d 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -494,12 +494,11 @@ namespace build2 { // Did the user ask us to use config.guess? // - string orig ( - ops.config_guess_specified () - ? run (3, - ops.config_guess (), - [](string& l, bool) {return move (l);}) - : BUILD2_HOST_TRIPLET); + string orig (config_guess + ? run (3, + *config_guess, + [](string& l, bool) {return move (l);}) + : BUILD2_HOST_TRIPLET); l5 ([&]{trace << "original host: '" << orig << "'";}); diff --git a/build2/depdb.ixx b/build2/depdb.ixx index 80a2ea6..cf67434 100644 --- a/build2/depdb.ixx +++ b/build2/depdb.ixx @@ -23,11 +23,7 @@ namespace build2 inline bool depdb:: mtime_check () { - // Note: options were validated in main(). - // - return (ops. mtime_check () ? true : - ops.no_mtime_check () ? false : - BUILD2_MTIME_CHECK); + return mtime_check_option ? *mtime_check_option : BUILD2_MTIME_CHECK; } inline void depdb:: diff --git a/build2/diagnostics.cxx b/build2/diagnostics.cxx index 3eb1e9a..ac95c64 100644 --- a/build2/diagnostics.cxx +++ b/build2/diagnostics.cxx @@ -12,6 +12,28 @@ using namespace std; namespace build2 { + // Diagnostics state (verbosity level, progress, etc). Keep disabled until + // set from options. + // + uint16_t verb = 0; + + optional diag_progress_option; + + bool diag_no_line = false; + bool diag_no_column = false; + + bool stderr_term = false; + + void + init_diag (uint16_t v, optional p, bool nl, bool nc, bool st) + { + verb = v; + diag_progress_option = p; + diag_no_line = nl; + diag_no_column = nc; + stderr_term = st; + } + // Stream verbosity. // const int stream_verb_index = ostream::xalloc (); @@ -29,10 +51,6 @@ namespace build2 r << butl::process_args {args, n}; } - // Diagnostics verbosity level. - // - uint16_t verb = 0; // Keep disabled until set from options. - // Diagnostics stack. // #ifdef __cpp_thread_local @@ -69,13 +87,13 @@ namespace build2 { r << *loc_.file << ':'; - if (!ops.no_line ()) + if (!diag_no_line) { if (loc_.line != 0) { r << loc_.line << ':'; - if (!ops.no_column ()) + if (!diag_no_column) { if (loc_.column != 0) r << loc_.column << ':'; diff --git a/build2/diagnostics.hxx b/build2/diagnostics.hxx index 39659c2..992e741 100644 --- a/build2/diagnostics.hxx +++ b/build2/diagnostics.hxx @@ -155,8 +155,9 @@ namespace build2 inline bool show_progress (uint16_t max_verb) { - return ops.progress () || - (stderr_term && verb >= 1 && verb <= max_verb && !ops.no_progress ()); + return diag_progress_option + ? *diag_progress_option + : stderr_term && verb >= 1 && verb <= max_verb; } // Diagnostic facility, base infrastructure. diff --git a/build2/function.test.cxx b/build2/function.test.cxx index a9ba7bb..b890bcd 100644 --- a/build2/function.test.cxx +++ b/build2/function.test.cxx @@ -36,7 +36,10 @@ namespace build2 int main (int, char* argv[]) { - init (argv[0], 1); // Fake build system driver, default verbosity. + // Fake build system driver, default verbosity. + // + init_diag (1); + init (argv[0]); reset (strings ()); // No command line variables. function_family f ("dummy"); diff --git a/build2/operation.cxx b/build2/operation.cxx index 9bc8a4e..0144e51 100644 --- a/build2/operation.cxx +++ b/build2/operation.cxx @@ -271,7 +271,7 @@ namespace build2 // Set the dry-run flag. // - dry_run = ops.dry_run (); + dry_run = dry_run_option; // Setup progress reporting if requested. // diff --git a/build2/test/script/parser.test.cxx b/build2/test/script/parser.test.cxx index 1862f98..ea5da0a 100644 --- a/build2/test/script/parser.test.cxx +++ b/build2/test/script/parser.test.cxx @@ -151,7 +151,10 @@ namespace build2 { tracer trace ("main"); - init (argv[0], 1); // Fake build system driver, default verbosity. + // Fake build system driver, default verbosity. + // + init_diag (1); + init (argv[0]); sched.startup (1); // Serial execution. reset (strings ()); // No command line variables. diff --git a/build2/utility.cxx b/build2/utility.cxx index 91aecdf..9448c03 100644 --- a/build2/utility.cxx +++ b/build2/utility.cxx @@ -69,13 +69,16 @@ namespace build2 // // // - - options ops; process_path argv0; - bool stderr_term; const standard_version build_version (BUILD2_VERSION_STR); + bool dry_run_option; + optional mtime_check_option; + + optional config_sub; + optional config_guess; + void check_build_version (const standard_version_constraint& c, const location& l) { @@ -483,23 +486,20 @@ namespace build2 } void - init (const char* a0, uint16_t v) + init (const char* a0, + bool kg, bool dr, optional mc, + optional cs, optional cg) { // Build system driver process path. // argv0 = process::path_search (a0, true); - // Diagnostics verbosity. - // - verb = v; + keep_going = kg; + dry_run_option = dr; + mtime_check_option = mc; - // Initialize time conversion data that is used by localtime_r(). - // -#ifndef _WIN32 - tzset (); -#else - _tzset (); -#endif + config_sub = move (cs); + config_guess = move (cg); // Figure out work and home directories. // diff --git a/build2/utility.hxx b/build2/utility.hxx index 0f7e521..3843528 100644 --- a/build2/utility.hxx +++ b/build2/utility.hxx @@ -21,7 +21,6 @@ #include #include -#include // "Fake" version values used during bootstrap. // @@ -82,11 +81,45 @@ namespace build2 using butl::eof; + // Diagnostics state (verbosity level, etc; see diagnostics.hxx). + // + // Note on naming of values (here and in the global state below) that come + // from the command line options: if a value is not meant to be used + // directly, then it has the _option suffix and a function or another + // variable as its public interface. + + // Initialize the diagnostics state. Should be called once early in main(). + // Default values are for unit tests. + // + void + init_diag (uint16_t verbosity, + optional progress = nullopt, + bool no_lines = false, + bool no_columns = false, + bool stderr_term = false); + + extern uint16_t verb; + const uint16_t verb_never = 7; + + extern optional diag_progress_option; // --[no-]progress + + extern bool diag_no_line; // --no-line + extern bool diag_no_column; // --no-column + extern bool stderr_term; // True if stderr is a terminal. - // Command line options. + // Global state (verbosity, home/work directories, etc). + + // Initialize the global state. Should be called once early in main(). + // Default values are for unit tests. // - extern options ops; + void + init (const char* argv0, + bool keep_going = false, + bool dry_run = false, + optional mtime_check = nullopt, + optional config_sub = nullopt, + optional config_guess = nullopt); // Build system driver process path (argv0.initial is argv[0]). // @@ -96,6 +129,12 @@ namespace build2 // extern const standard_version build_version; + extern bool dry_run_option; // --dry-run + extern optional mtime_check_option; // --[no-]mtime-check + + extern optional config_sub; // --config-sub + extern optional config_guess; // --config-guess + class location; void @@ -136,11 +175,6 @@ namespace build2 string diag_relative (const path&, bool current = true); - // Diagnostics forward declarations (see diagnostics.hxx). - // - extern uint16_t verb; - const uint16_t verb_never = 7; - // Basic process utilities. // // The run*() functions with process_path assume that you are printing @@ -621,12 +655,6 @@ namespace build2 // string apply_pattern (const char* stem, const string* pattern); - - // Initialize build2 global state (verbosity, home/work directories, etc). - // Should be called early in main() once. - // - void - init (const char* argv0, uint16_t verbosity); } #include -- cgit v1.1