From 125fa983abc538c8913e4c5f2381becaee89a5f3 Mon Sep 17 00:00:00 2001 From: Alexandre Duret-Lutz Date: Fri, 20 Mar 2015 19:07:02 +0100 Subject: [PATCH] Do not store getenv() pointers in static variables. ... or the pointer might be invalidated if the environments changes. Fixes #63. * src/taalgos/dotty.cc, src/tgbaalgos/dotty.cc, src/tgbaalgos/dtbasat.cc, src/tgbaalgos/dtgbasat.cc: Copy the environment in strings instead. * wrap/python/tests/automata.ipynb: Adjust comment. --- src/taalgos/dotty.cc | 14 +++++++++++-- src/tgbaalgos/dotty.cc | 28 ++++++++++++++++++++------ src/tgbaalgos/dtbasat.cc | 13 +++++++++--- src/tgbaalgos/dtgbasat.cc | 13 +++++++++--- wrap/python/tests/automata.ipynb | 34 ++++++++++++++++---------------- 5 files changed, 71 insertions(+), 31 deletions(-) diff --git a/src/taalgos/dotty.cc b/src/taalgos/dotty.cc index 6ba8ba802..8cabf0772 100644 --- a/src/taalgos/dotty.cc +++ b/src/taalgos/dotty.cc @@ -41,8 +41,18 @@ namespace spot { os_ << "digraph G {\n"; - static const char* extra = getenv("SPOT_DOTEXTRA"); - if (extra) + // Always copy the environment variable into a static string, + // so that we (1) look it up once, but (2) won't crash if the + // environment is changed. + static std::string extra = []() + { + auto s = getenv("SPOT_DOTEXTRA"); + return s ? s : ""; + }(); + // Any extra text passed in the SPOT_DOTEXTRA environment + // variable should be output at the end of the "header", so + // that our setup can be overridden. + if (!extra.empty()) os_ << " " << extra << '\n'; artificial_initial_state_ = t_automata_->get_artificial_initial_state(); diff --git a/src/tgbaalgos/dotty.cc b/src/tgbaalgos/dotty.cc index 1b7bf9d30..27eb82af8 100644 --- a/src/tgbaalgos/dotty.cc +++ b/src/tgbaalgos/dotty.cc @@ -88,13 +88,22 @@ namespace spot { case '.': { - static const char* def = getenv("SPOT_DOTDEFAULT"); + // Copy the value in a string, so future calls to + // parse_opts do not fail if the environment has + // changed. (This matters particularly in an ipython + // notebook, where it is tempting to redefine + // SPOT_DOTDEFAULT.) + static std::string def = []() + { + auto s = getenv("SPOT_DOTDEFAULT"); + return s ? s : ""; + }(); // Prevent infinite recursions... - if (orig == def) + if (orig == def.c_str()) throw std::runtime_error (std::string("SPOT_DOTDEFAULT should not contain '.'")); - if (def) - parse_opts(def); + if (!def.empty()) + parse_opts(def.c_str()); break; } case 'a': @@ -269,11 +278,18 @@ namespace spot << "\"\n node [fontname=\"" << opt_font_ << "\"]\n edge [fontname=\"" << opt_font_ << "\"]\n"; + // Always copy the environment variable into a static string, + // so that we (1) look it up once, but (2) won't crash if the + // environment is changed. + static std::string extra = []() + { + auto s = getenv("SPOT_DOTEXTRA"); + return s ? s : ""; + }(); // Any extra text passed in the SPOT_DOTEXTRA environment // variable should be output at the end of the "header", so // that our setup can be overridden. - static const char* extra = getenv("SPOT_DOTEXTRA"); - if (extra && *extra) + if (!extra.empty()) os_ << " " << extra << '\n'; os_ << " I [label=\"\", style=invis, "; os_ << (opt_horizontal_ ? "width" : "height"); diff --git a/src/tgbaalgos/dtbasat.cc b/src/tgbaalgos/dtbasat.cc index 9dcc87e3a..8d53da684 100644 --- a/src/tgbaalgos/dtbasat.cc +++ b/src/tgbaalgos/dtbasat.cc @@ -766,8 +766,15 @@ namespace spot if (!solution.second.empty()) res = sat_build(solution.second, d, a, state_based); - static const char* log = getenv("SPOT_SATLOG"); - if (log) + // Always copy the environment variable into a static string, + // so that we (1) look it up once, but (2) won't crash if the + // environment is changed. + static std::string log = []() + { + auto s = getenv("SPOT_SATLOG"); + return s ? s : ""; + }(); + if (!log.empty()) { std::fstream out(log, std::ios_base::app | std::ios_base::out); @@ -790,7 +797,7 @@ namespace spot << te.utime() << ',' << te.stime() << ',' << ts.utime() << ',' << ts.stime() << '\n'; } - static const char* show = getenv("SPOT_SATSHOW"); + static bool show = getenv("SPOT_SATSHOW"); if (show && res) dotty_reachable(std::cout, res); diff --git a/src/tgbaalgos/dtgbasat.cc b/src/tgbaalgos/dtgbasat.cc index 230a67f62..6c1c9d61e 100644 --- a/src/tgbaalgos/dtgbasat.cc +++ b/src/tgbaalgos/dtgbasat.cc @@ -906,8 +906,15 @@ namespace spot if (!solution.second.empty()) res = sat_build(solution.second, d, a, state_based); - static const char* log = getenv("SPOT_SATLOG"); - if (log) + // Always copy the environment variable into a static string, + // so that we (1) look it up once, but (2) won't crash if the + // environment is changed. + static std::string log = []() + { + auto s = getenv("SPOT_SATLOG"); + return s ? s : ""; + }(); + if (!log.empty()) { std::fstream out(log, std::ios_base::app | std::ios_base::out); @@ -930,7 +937,7 @@ namespace spot << te.utime() << ',' << te.stime() << ',' << ts.utime() << ',' << ts.stime() << '\n'; } - static const char* show = getenv("SPOT_SATSHOW"); + static bool show = getenv("SPOT_SATSHOW"); if (show && res) dotty_reachable(std::cout, res); diff --git a/wrap/python/tests/automata.ipynb b/wrap/python/tests/automata.ipynb index 3cca8794b..40ac404a8 100644 --- a/wrap/python/tests/automata.ipynb +++ b/wrap/python/tests/automata.ipynb @@ -1,7 +1,7 @@ { "metadata": { "name": "", - "signature": "sha256:9b116e5c61ed241183bd5fb9cd5e35010cceb5043c2fab2be81473f829bc528e" + "signature": "sha256:c0ee6809405cd6426445a43cea6fc3448a99423e1a4be95efeda2fa03680f05f" }, "nbformat": 3, "nbformat_minor": 0, @@ -13,10 +13,10 @@ "collapsed": false, "input": [ "import os\n", - "# Note that Spot (loaded by the kernel) will cache a \n", - "# pointer to the environment variable the first time it\n", - "# reads them. The Kernel may crash if you attempt to\n", - "# modify the variables afterwards. See issue #63.\n", + "# Note that Spot (loaded by the kernel) will store a copy of\n", + "# the environment variable the first time it reads them, so\n", + "# if you change those variables, the new values will be ignored\n", + "# until you restart the kernel.\n", "os.environ['SPOT_DOTEXTRA'] = 'size=\"11,5\" node[style=filled,fillcolor=\"#ffffaa\"]'\n", "os.environ['SPOT_DOTDEFAULT'] = 'rbcf(Lato)'\n", "import spot" @@ -159,7 +159,7 @@ "\n" ], "text": [ - " *' at 0x7f17fc124150> >" + " *' at 0x7fa0e8162150> >" ] } ], @@ -291,7 +291,7 @@ "" ], "text": [ - "" + "" ] } ], @@ -437,7 +437,7 @@ "" ], "text": [ - "" + "" ] } ], @@ -530,7 +530,7 @@ "\n" ], "text": [ - " *' at 0x7f17fc0fb930> >" + " *' at 0x7fa0e8138930> >" ] } ], @@ -600,7 +600,7 @@ "\n" ], "text": [ - " *' at 0x7f17fc0fbb70> >" + " *' at 0x7fa0e8138b70> >" ] } ], @@ -669,7 +669,7 @@ "\n" ], "text": [ - " *' at 0x7f17fc0fb990> >" + " *' at 0x7fa0e81389c0> >" ] } ], @@ -785,7 +785,7 @@ "" ], "text": [ - "" + "" ] } ], @@ -975,7 +975,7 @@ "" ], "text": [ - "" + "" ] } ], @@ -1068,7 +1068,7 @@ "\n" ], "text": [ - " *' at 0x7f17fc0fbb40> >" + " *' at 0x7fa0e81389f0> >" ] } ], @@ -1086,7 +1086,7 @@ { "metadata": {}, "output_type": "pyout", - "prompt_number": 15, + "prompt_number": 13, "svg": [ "\n", "\n" ], "text": [ - " *' at 0x7f17fc0fb9c0> >" + " *' at 0x7fa0e8138b40> >" ] } ], - "prompt_number": 15 + "prompt_number": 13 } ], "metadata": {}