bugfix: avoid naive use of 'putenv' by providing 'FbTk::App::setenv()'

to quote from 'man putenv':

   The string pointed to by string becomes part of the environment,
   so altering the string changes the environment.

so, using putenv like

   {
      std::string foo("FOO=bar");
      putenv(foo.c_str());
   }

is wrong and leads to a potentially corrupted environment. valgrind
complaint correctly.

FbTk::App seems to be the appropriate place to hold '::seten()'
because it alters the environment of the application.
This commit is contained in:
Mathias Gumz 2010-09-17 23:43:24 +02:00
parent f3ad09c4ce
commit 87b45bd0d1
4 changed files with 66 additions and 36 deletions

View file

@ -142,8 +142,7 @@ int ExecuteCmd::run() {
if (pid) if (pid)
return pid; return pid;
string displaystring("DISPLAY="); string display = DisplayString(FbTk::App::instance()->display());
displaystring += DisplayString(FbTk::App::instance()->display());
int screen_num = m_screen_num; int screen_num = m_screen_num;
if (screen_num < 0) { if (screen_num < 0) {
if (Fluxbox::instance()->mouseScreen() == 0) if (Fluxbox::instance()->mouseScreen() == 0)
@ -158,12 +157,11 @@ int ExecuteCmd::run() {
if (!shell) if (!shell)
shell = "/bin/sh"; shell = "/bin/sh";
// remove last number of display and add screen num display.erase(display.size()-1);
displaystring.erase(displaystring.size()-1); display += FbTk::StringUtil::number2String(screen_num);
displaystring += FbTk::StringUtil::number2String(screen_num);
setsid(); setsid();
putenv(const_cast<char *>(displaystring.c_str())); FbTk::App::setenv("DISPLAY", display.c_str());
execl(shell, shell, "-c", m_cmd.c_str(), static_cast<void*>(NULL)); execl(shell, shell, "-c", m_cmd.c_str(), static_cast<void*>(NULL));
exit(EXIT_SUCCESS); exit(EXIT_SUCCESS);
@ -197,34 +195,7 @@ ExportCmd::ExportCmd(const string& name, const string& value) :
void ExportCmd::execute() { void ExportCmd::execute() {
// the setenv()-routine is not everywhere available and FbTk::App::instance()->setenv(m_name.c_str(), m_value.c_str());
// putenv() doesnt manage the strings in the environment
// and hence we have to do that on our own to avoid memleaking
static set<char*> stored;
char* newenv = new char[m_name.size() + m_value.size() + 2];
if (newenv) {
char* oldenv = getenv(m_name.c_str());
// oldenv points to the value .. we have to go back a bit
if (oldenv && stored.find(oldenv - (m_name.size() + 1)) != stored.end())
oldenv -= (m_name.size() + 1);
else
oldenv = NULL;
memset(newenv, 0, m_name.size() + m_value.size() + 2);
strcat(newenv, m_name.c_str());
strcat(newenv, "=");
strcat(newenv, m_value.c_str());
if (putenv(newenv) == 0) {
if (oldenv) {
stored.erase(oldenv);
delete[] oldenv;
}
stored.insert(newenv);
}
}
} }
REGISTER_COMMAND(exit, FbCommands::ExitFluxboxCmd, void); REGISTER_COMMAND(exit, FbCommands::ExitFluxboxCmd, void);

View file

@ -26,6 +26,21 @@
#include "EventManager.hh" #include "EventManager.hh"
#ifdef HAVE_CSTRING
#include <cstring>
#else
#include <string.h>
#endif
#ifdef HAVE_CSTDLIB
#include <cstdlib>
#else
#include <stdlib.h>
#endif
#include <set>
namespace FbTk { namespace FbTk {
App *App::s_app = 0; App *App::s_app = 0;
@ -79,4 +94,44 @@ void App::end() {
m_done = true; //end loop in App::eventLoop m_done = true; //end loop in App::eventLoop
} }
bool App::setenv(const char* key, const char* value) {
if (!key || !*key)
return false;
static std::set<char*> stored;
const size_t key_size = strlen(key);
const size_t value_size = value ? strlen(value) : 0;
char* newenv = new char[key_size + value_size + 2];
if (newenv) {
char* oldenv = getenv(key);
// oldenv points to the value .. we have to go back a bit
if (oldenv && stored.find(oldenv - (key_size + 1)) != stored.end())
oldenv -= (key_size + 1);
else
oldenv = NULL;
memset(newenv, 0, key_size + value_size + 2);
strcat(newenv, key);
strcat(newenv, "=");
if (value_size > 0)
strcat(newenv, value);
if (putenv(newenv) == 0) {
if (oldenv) {
stored.erase(oldenv);
delete[] oldenv;
}
stored.insert(newenv);
}
return true;
}
return false;
}
} // end namespace FbTk } // end namespace FbTk

View file

@ -53,6 +53,11 @@ public:
/// forces an end to event loop /// forces an end to event loop
void end(); void end();
bool done() const { return m_done; } bool done() const { return m_done; }
// the setenv()-routine is not everywhere available and
// putenv() doesnt manage the strings in the environment
// and hence we have to do that on our own to avoid memleaking
static bool setenv(const char* key, const char* value);
private: private:
static App *s_app; static App *s_app;
bool m_done; bool m_done;

View file

@ -219,8 +219,7 @@ static void parseOptions(int argc, char** argv, Options& opts) {
} }
opts.session_display = argv[i]; opts.session_display = argv[i];
string display_env = "DISPLAY=" + opts.session_display; if (!FbTk::App::setenv("DISPLAY", argv[i])) {
if (putenv(const_cast<char *>(display_env.c_str()))) {
cerr<<_FB_CONSOLETEXT(main, WarnDisplayEnv, cerr<<_FB_CONSOLETEXT(main, WarnDisplayEnv,
"warning: couldn't set environment variable 'DISPLAY'", "warning: couldn't set environment variable 'DISPLAY'",
"")<<endl; "")<<endl;