Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Implementation of platform-independ environment manipulations #1272

Merged
merged 2 commits into from

7 participants

@Karlson2k
Collaborator

While debugging of XBMC, I've discovered that win32 environment manipulation isn't working at all.
The problem is that win32 functions SetEnvironmentVariable/GetEnvironmentVariable and runtime functions getenv/putenv operate with separate copies of Environment. So after set (for example) of 'XBMC_HOME' with SetEnvironmentVariable, next call to getenv for 'XBMC_HOME' returns error (variable not set).
Other problem on win32 platform, that XBMC use UTF-8 for CStdString, but when it converted to 'char *' win32 use it as ANSI or OEM encoding. That can cause problems if variable value contains some national characters, as paths variables.
One more problem for win32 - different runtime libraries for third-party modules, which should be updated separately. This is done partially by win32env.c, but that implementation is buggy (new loaded libraries isn't checked, can potentially crash after library unload).
This implementation is uniform for all platforms, so we can drop a lot of "ifdef"s in code.
All UTF-8<->wide conversion is carefully implemented internally where is't needed.
Almost all strings forms used in XBMC is supported as arguments.

This PR is adds new functions support and only a few function uses (as a test), and only for win32 platform.
If it's OK, I'll create more PRs with total replace for setenv/getenv functions.

@wsoltys
Collaborator

I like the idea in general but I dunno why
1) we need so much code and so many new functions
2) charsetconverter isn't used

For 2) needs to be taken care of as we use it everywhere and if there's something wrong for windows it should be fixed there. Also please replace _WIN32 by TARGET_WINDOWS.
At last I dunno if Linux/OSX boys are willing to add extra code for existing functions on their platform even though we might get rid of some ifdefs ;)

@Karlson2k
Collaborator

@wsoltys Answers: :)
1) Basically I was going to implement only 'char ', 'wchar_t *', 'CStdStringA' and 'CStdStringW' versions. All those type of variables are used in XBMC and some function need internally CStdStringX versions, some - only Xchar version, so to avoid unnecessary char<->CStdString conversions I implement 4 versions of functions.
But then I got an error when I tried to compile a simple command like "xbmc_setenv("XBMC_HOME", cstdstringvar). Compiler didn't know which functions should be used "xbmc_setenv(char *, char *)" or "xbmc_setenv(CStdStringA&, CStdStringA&)" as both conversion is possible. So I decided to implement all possible versions, trying each time to use less possible conversions.
Anyway, this give a great flexibility on using this functions and it's not too hard to maintain all those code as huge part of functions is implemented by calling other similar functions.

2) charsetconverter is used for non-windows platform. But charset conversion is needed mostly for Windows. So I decided to use direct Windows functions for two reasons:
1. When coding for Windows, native Windows conversion should be faster.
2. This is the only way to check conversion success, as XBMC conversion functions in case of fail just return original string converted via CStdString conversion (so converted as ANSI -> wide or vice versa). In case that string contain illegal ANSI characters, returned string can be just empty so if 'value' parameter conversion failed, then xbmc_envset can delete variable instead of setting new value for it.

_WIN32 is replaced.
And of course, it make sense to update only multiplatfrom code. Pure Linux/OSX code should use their native functions.

@jmarshallnz
Owner

Before we even get into the code, where is the bug?

@Karlson2k
Collaborator

At least one bug here, in Application.cpp:

CUtil::GetHomePath(xbmcPath);
SetEnvironmentVariable("XBMC_HOME", xbmcPath.c_str());

We are trying to set "XBMC_HOME", so next call to CUtil::GetHomePath should use this environment variable instead of full detection. But "GetHomePath" uses runtime function "_wgetenv" (which one operates with separate environment copy) and never get this variable, so every time "GetHomePath" uses full detection again.
I'm sure that in XBMC a lot of places where different environment is used. For example first setting "OS" variable will be ignored by any external libs, like python as it compiled by VS2008.

As I see, XBMC code uses widely "SetEnvironmentVariable" (as it simpler than "putenv") for setting variables, but uses "getenv" for reading it (as it simpler than "GetEnvironmentVariable"). But it's wrong, as "getenv" will not see changes made by "SetEnvironmentVariable" and vice versa.

To properly change variable on win32 platform you need to update it in:

  • runtime environment (by "putenv")
  • process environment (by "SetEnvironmentVariable", this environment is inherited by child processes )
  • other runtime versions environment used by third-party libs (each runtime uses independ copy of environment)

Currently XBMC uses in some places "win32env.c" to update environment for python, but "win32env.c" is buggy and could crash after library unloading.

If this code go to XBMC, we will need to get rid of "win32env.c".

@jmarshallnz
Owner

Given that win32env.cpp comes directly out of PostgreSQL, I'd suggest if there are any bugs in that code, you should direct them to PostgreSQL first and foremost. Perhaps any bugs are really only due to it's use in XBMC?

If you want this accepted, remove all MultiByteToWideChar stuff, get rid of everything except char* (utf8) versions of put/get/set/unset designed so they work exactly like posix where doable.

Once the trees are cleared we might be able to see the forest.

@Karlson2k
Collaborator

@jmarshallnz May be PostgreSQL don't unload libs at runtime - I don't know.
I just want to implement convenient functions for XBMC with XBMC's specific.

I removed all win32 charset conversions and split code to several commits.
4c5bd47 contains main logic, but main logic includes some CStdStringA and CStdStringW functions. The main reasons for it to explicitly return a copy of variable value (to not confuse with posix behavor as posix specify return value of getenv as modifiable, but this not possible on Windows platform with wide<->UTF-8 conversions). And CStdString is used for putenv (again - posix specify that string should be putted as later modifiable by it pointer and again it's not possible on windows).
CStdStringW is a part of main logic as some parts of XBMC use wide chars, so Environment can be modified without unnecessary wide->UTF8->wide conversions.

convertFromUtf8 and convertToUtf8 is just a wrappers for charsetconverter with additional checks and shortcuts useful mainly to Environment functions.

@Karlson2k
Collaborator

@jmarshallnz @wsoltys
PR is still waiting.
Just check e777e57 at first.
This could solve number of problems and potential problems.
I'm still not sure about charsetconverter as SetEnv can be used before it initialization. May be implement win32 charset conversion at least as fallback?

@jmarshallnz
Owner
@Karlson2k
Collaborator

@jmarshallnz OK, wide char is really rare.
About "char *" - I just want to avoid extra char -> CStdString -> char conversion.
And the most common usage for setenv is "setenv("envvar", varvalue)", so if we accept "char *" we'll just get a pointer without any initialization/conversion/c_str.
I'll start with dropping of wide char.

@jmarshallnz
Owner

I know, thus the suggest to accept a const std::string & which will quite happily take both.

@Karlson2k
Collaborator

@jmarshallnz Rewritten.
Now uses mostly std::string and std::wstring.

@Karlson2k
Collaborator

@jmarshallnz What can I do to improve this PR even more? :)

@Karlson2k
Collaborator

@jmarshallnz @wsoltys
Is it time?

@jmarshallnz
Owner

XBMC does everything in utf8. Drop all wstring functions as they're not needed.

Under which circumstances is the charset convertor not available?

@wsoltys
Collaborator

Apparently a wchar version was needed: c5f50fb

@jmarshallnz
Owner
@wsoltys
Collaborator

@chadoe can you tell us why the UTF8->wchar conversion was needed?

@chadoe
Collaborator

I didn't read everything here but for the change @wsoltys is referring to:
Because there's the ansi and the wide putenv api function... and special character paths stored in utf8 converted to ansi could result in loss of characters with certain characters that users use in their windows username. Which python would then reading back from the environment (invalid paths).
But maybe some of the many later fixes makes using the wide api obsolete, I remember making use of system codepages for converting the paths later on...

@Karlson2k
Collaborator

@wsoltys @jmarshallnz wchar version is mostly for win32-only code sections. It's easy to use wchar in win32 as windows doesn't support UTF-8 as input/output (except charset conversion functions).

@jmarshallnz XBMC's charset converter is not available before initialization / if shutdown in progress. Same as for CLog: https://github.com/xbmc/xbmc/blob/master/xbmc/utils/log.cpp#L226

@jmarshallnz
Owner

Is the environment stuff even used before initialization or after shutdown? If it is, then I'd tend to use the win32 API entirely for the utf8->wide conversion. If it's not, use the charsetconverter everywhere. Mixing and matching doesn't make much sense.

XBMC doesn't use a wchar version of set/putenv at all so requires only a utf8 version (obviously wrapped to the wide API). There's no point having wrapping code that isn't used.

@Karlson2k
Collaborator

Wchar is used only in https://github.com/xbmc/xbmc/blob/master/xbmc/win32/WIN32Util.cpp#L503 and https://github.com/xbmc/xbmc/blob/master/xbmc/win32/WIN32Util.cpp#L513, so wrappers can be really removed.
And I'll check initialization/deinitialization order, but it can be changed in future (or will require environment manipulation in different moment). As utf8<->wchar conversion needed only for win32 platform, it will be more future proof to use win32 conversion (and no conversion for other platforms).

Should I move all function to some class (XBMCEnv?) or to another namespace?

@jmarshallnz
Owner

Yeah, agreed - just use the win32 API for conversion to keep it simple.

It might be useful to wrap in a (static?) class I guess. If so, keep the name of the functions simple, corresponding to the name of the posix function (SetEnv, PutEnv etc.)

@Karlson2k
Collaborator

@jmarshallnz Rebased, all public wchar_t/wstring functions removed, WStdString usage removed, everything moved into static class, some functions was refactored, some rewritten.
Makefile updated. Need help on updating Mac projects.

xbmc/utils/XbmcEnv.cpp
((212 lines not shown))
+ int varSize = GetEnvironmentVariableW(Wname.c_str(), NULL, 0);
+ if (varSize == 0)
+ return ""; // Not found
+ wchar_t * valBuf = new wchar_t[varSize];
+ if (GetEnvironmentVariableW(Wname.c_str(), valBuf, varSize) != varSize-1)
+ {
+ delete[] valBuf;
+ return "";
+ }
+ Wvalue = valBuf;
+ delete[] valBuf;
+
+ return CXbmcEnv::win32ConvertWToUtf8(Wvalue.c_str());
+#else
+ return std::string(::getenv(name.c_str()));
+#endif
@jmarshallnz Owner

do you actually need the std::string() wrapper?

@Karlson2k Collaborator

Not really. It'll be used by compiler in any case.
I use it for clarity of transforming result type.
If you say that readability is better without it, I'll remove it.

@jmarshallnz Owner

IMO it's better without.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.cpp
((209 lines not shown))
+
+ // Not found in Environment of runtime library
+ // Try Environment of process as fallback
+ int varSize = GetEnvironmentVariableW(Wname.c_str(), NULL, 0);
+ if (varSize == 0)
+ return ""; // Not found
+ wchar_t * valBuf = new wchar_t[varSize];
+ if (GetEnvironmentVariableW(Wname.c_str(), valBuf, varSize) != varSize-1)
+ {
+ delete[] valBuf;
+ return "";
+ }
+ Wvalue = valBuf;
+ delete[] valBuf;
+
+ return CXbmcEnv::win32ConvertWToUtf8(Wvalue.c_str());
@jmarshallnz Owner

No need to prefix with CXbmcEnv

@Karlson2k Collaborator

That's for consistency with CXbmcEnv::setenv (and other member function).
And again, for clarity - used local helper function.
But I will not insist. Remove?

@jmarshallnz Owner

Remove them all. We already know which class we're in, there's no need to tell the compiler. The :: on the functions that are global make it clear they're global.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.cpp
((232 lines not shown))
+#ifdef TARGET_WINDOWS
+ return (win32_setenv(name, "", deleteVariable)) == 0 ? 0 : -1;
+#else
+ return ::unsetenv(name);
+#endif
+}
+
+int CXbmcEnv::putenv(const std::string &envstring)
+{
+ if (envstring.empty())
+ return 0;
+ int pos = envstring.find('=');
+ if (pos == 0) // '=' is the first character
+ return -1;
+ if (pos == std::string::npos)
+ return CXbmcEnv::unsetenv(envstring);
@jmarshallnz Owner

No need to prefix with CXbmcEnv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.cpp
@@ -0,0 +1,258 @@
+/**
+ * \file utils\XbncEnv.cpp
+ * \brief Implements xbmc environment manipulations functions.
+ *
+ * Some ideas was inspired by 'src/port/win32env.c' file.
+ * Refined, updated, enhanced and modified for XBMC by Karlson2k.
+ */
+
+/*
+ * Copyright (C) 2012 Team XBMC
@jmarshallnz Owner

Update copyright

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.cpp
@@ -0,0 +1,258 @@
+/**
+ * \file utils\XbncEnv.cpp
+ * \brief Implements xbmc environment manipulations functions.
+ *
+ * Some ideas was inspired by 'src/port/win32env.c' file.
+ * Refined, updated, enhanced and modified for XBMC by Karlson2k.
+ */
@jmarshallnz Owner

Move to underneath copyright message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.cpp
((51 lines not shown))
+ if (bufSize == 0)
+ return L"";
+ wchar_t *converted = new wchar_t[bufSize];
+ if (MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, text, -1, converted, bufSize) != bufSize)
+ {
+ delete[] converted;
+ return L"";
+ }
+
+ std::wstring Wret (converted);
+ delete[] converted;
+
+ if (resultSuccessful != NULL)
+ *resultSuccessful = true;
+ return Wret;
+}
@jmarshallnz Owner

This functions might work better as:

bool win32ConvertUtf8ToW(const char *text, std::wstring &result)

as you tend to check the return code rather than the resulting string?

The next one you tend to use unchecked (as that makes most sense) so it doesn't matter so much, but they should probably be consistent.

@Karlson2k Collaborator

Need to check result only in one or two places (to avoid accidental variable deletion instead of modifying). In most cases only string result is needed.
But in that form I loose ability to easy use it in form like

return win32ConvertWToUtf8(somfunc(win32ConvertUtf8ToW(str)));

With bool return this nice code is transformed into

std::wstring wstr;
win32ConvertUtf8ToW(str, wstr);
std::string strRet;
win32ConvertWToUtf8(somefunc(wstr), strRet);
return strRet;

I could replace pointer to bool with reference to bool. Than function can use it without check for NULL.

@jmarshallnz Owner

You use the resulting string from WToUtf8 in this way once (the only time it's used, so you never check the return code boolean). You use the bool from Utf8ToW as a check 2 out of 3 times, and the 3rd time you do a check based on the emptiness of the returned string (which is OK in that case).

It doesn't really matter either way - just something I noticed that looked a bit ugly. :)

The problem with reference to bool is you can't ignore it completely (when you don't care about errors) without having an ugly wrapper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
project/VS2010Express/XBMC.vcxproj.filters
@@ -5877,4 +5883,4 @@
<Filter>interfaces\swig</Filter>
</None>
</ItemGroup>
-</Project>
+</Project>
@MartijnKaijser Owner

This line end won't cause issues? Same in other project file.

@Karlson2k Collaborator

No issues. It's modified by VS and XML is loaded correct in any case.
But I'll add linebreak.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.cpp
((53 lines not shown))
+ wchar_t *converted = new wchar_t[bufSize];
+ if (MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, text, -1, converted, bufSize) != bufSize)
+ {
+ delete[] converted;
+ return L"";
+ }
+
+ std::wstring Wret (converted);
+ delete[] converted;
+
+ if (resultSuccessful != NULL)
+ *resultSuccessful = true;
+ return Wret;
+}
+
+std::string CXbmcEnv::win32ConvertWToUtf8(const wchar_t *text, bool *resultSuccessful /*= NULL*/)
@jmarshallnz Owner

If you just use const wstring &text you don't have to check for NULL, and also don't need to .c_str() the input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.h
((7 lines not shown))
+ * + unsetenv
+ * + putenv
+ * + getenv
+ *
+ * Basically functions work like POSIX analogs, but with some exceptions:
+ * 1. CXbmcEnv::setenv can be used to unset variables. Just pass empty string for 'value' parameter.
+ * 2. CXbmcEnv::setenv parameter 'overwrite' is optional, set by default to 1 (allow overwrite)
+ * 3. CXbmcEnv::putenv uses copy of provided string (rather than string itself) to change environment.
+ * 4. CXbmcEnv::putenv can be used to unset variables. Set parameter to 'var=' (Windows style) or just 'var' (POSIX style), and 'var' will be unset.
+ * 5. CXbmcEnv::getenv returns a copy of environment variable value instead of pointer to value.
+ *
+ * All 'std::string' types are supposed to be in UTF-8 encoding.
+ * All functions work on all platforms. Special care is taken on Windows platform where Environment is changed for process itself,
+ * in own runtime library and in all runtime libraries (MSVCRT) loaded by third-party modules.
+ * Functions internally make all necessary UTF-8 <-> wide conversions.
+ */
@jmarshallnz Owner

Put this as the doxy on the class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.h
((1 lines not shown))
+/**
+ * \file utils\XbmcEnv.h
+ * \brief Declares the xbmc environment manipulations functions.
+ *
+ * Provide XBMC's analog for POSIX functions:
+ * + setenv
+ * + unsetenv
+ * + putenv
+ * + getenv
+ *
+ * Basically functions work like POSIX analogs, but with some exceptions:
+ * 1. CXbmcEnv::setenv can be used to unset variables. Just pass empty string for 'value' parameter.
+ * 2. CXbmcEnv::setenv parameter 'overwrite' is optional, set by default to 1 (allow overwrite)
+ * 3. CXbmcEnv::putenv uses copy of provided string (rather than string itself) to change environment.
+ * 4. CXbmcEnv::putenv can be used to unset variables. Set parameter to 'var=' (Windows style) or just 'var' (POSIX style), and 'var' will be unset.
+ * 5. CXbmcEnv::getenv returns a copy of environment variable value instead of pointer to value.
@jmarshallnz Owner

Which of these exceptions do we make use of, other than the copies rather than pointers to value? Perhaps it makes sense to just have the copies instead of the behaviour changes? I dunno :)

@Karlson2k Collaborator
  1. setenv with empty string is consistent with MS CRT method of deletion of variables. And doesn't contradict with POSIX.
  2. making 'overwrite' optional is simple convenient in C++, and in most cases overwrite is set to '1'.
  3. putenv use std::string, so it just clarification
  4. it's MS-style
  5. by POSIX environment can be changed using returned pointer. As std::string used, it's just clarification.

Actually, all point are more extending, rather than exceptions.
If it looks too complex, this notice can be removed, as functions can be used in POSIX-style.

@jmarshallnz Owner

Maybe just add a note to that effect? i.e. "You can generally use the functions as you would normally in POSIX-style. The differences below are just to make things more convenient through use of std::string (2,3,5), and to also allow the Win32 method of unsetting variables (1+4) if wanted."

@Karlson2k Collaborator

Yes, this is definitely better and simpler. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/utils/XbmcEnv.h
((82 lines not shown))
+ * \sa xbmc_getenvUtf8, xbmc_getenvW
+ */
+ static std::string getenv(const std::string &name);
+#ifdef TARGET_WINDOWS
+private:
+ static std::wstring win32ConvertUtf8ToW(const char *text, bool *resultSuccessful = NULL);
+ static std::string win32ConvertWToUtf8(const wchar_t *text, bool *resultSuccessful = NULL);
+ enum updateAction:int
+ {
+ addOrUpdateOnly = -2,
+ deleteVariable = -1,
+ addOnly = 0,
+ autoDetect = 1,
+ };
+ static int win32_setenv(const std::string &name, const std::string &value = "", updateAction action = autoDetect);
+#endif // TARGET_WINDOWS
@jmarshallnz Owner

These can probably just go in the .cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmarshallnz
Owner

Just a few cosmetic comments more than anything. One last one: I'd name it Environment.h/cpp (CEnvironment) - we don't need to know it's in Xbmc :)

@Karlson2k
Collaborator

@jmarshallnz Done + few more cosmetics.

xbmc/Application.cpp
@@ -344,6 +344,10 @@
#include "android/activity/XBMCApp.h"
#endif
+#ifdef TARGET_WINDOWS
+#include "utils/XbmcEnv.h"
@jmarshallnz Owner

Environment.h

@Karlson2k Collaborator

Thanks! Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmarshallnz jmarshallnz was assigned
@jmarshallnz
Owner

Looks good. Build needs testing on Linux, else drop the Makefile change. Will merge in May window.

@Karlson2k
Collaborator

Small correction for Linux (replace "pop_back" with "erase" for std::string). "pop_back" is part of C++11, but for it's available on MSVC C++98.
Tested build on Ubuntu v12.10 x32.

@Karlson2k
Collaborator

@jmarshallnz Is it OK for May window?

@Karlson2k
Collaborator

@Memphiz Needs sync xcode projects.

@Karlson2k
Collaborator

Fixed typo in comment

@Karlson2k
Collaborator

@jmarshallnz Postpone until next window?

@jmarshallnz
Owner

If you drop the Makefile change it shouldn't affect other platforms at all, right?

Or, if you've build tested on linux, the Makefile change can stay.

Either or, I'm happy to pull this in as soon as that is done (this window).

@Karlson2k
Collaborator

It's build tested on Linux (Ubuntu).
I think, it's fully ready to merge. :)

@jmarshallnz
Owner

Not according to github - needs rebase.

@Karlson2k
Collaborator

@jmarshallnz, it's ready again. :)

@jmarshallnz jmarshallnz merged commit 1fb487a into xbmc:master
@jmarshallnz
Owner

Thanks :)

@Karlson2k
Collaborator

@Memphiz Branch was merged, need xcode projects sync.

@Memphiz
Owner

@Karlson2k - done - i guess it needs codechanges somewhere to make use of it?

@Karlson2k
Collaborator

Those functions are mostly wrappers around system native function. Special tricks used only on win32.
So for non-win32 you can continue use system native functions in system specific code or use this class if it's more convenient.
No need to convert old system specific code.
Those functions must be used for win32 code and common code parts (for win32 and non-win32).
It's done in #2739.
@Memphiz, could your help on checking PR #2739 for ios/osx?

@mkortstiege

"unsigned int" in order to get rid of the sign-compare warnings in line 240 and 242.

Collaborator

Thanks!
Here: 9d68380 in #2739

@LongChair LongChair referenced this pull request from a commit in plexinc/plex-home-theater-public
@LongChair LongChair Fix Error "The reponse from the server did not make sense" #1272.
We shouldn't display that error message to the GUI. It's been replaced by an error log message as requested.
b37399e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 11, 2013
  1. @Karlson2k
  2. @Karlson2k

    Environment: Implement "CEnvironment" class with functions for enviro…

    Karlson2k authored
    …nment manipulation and add it files to project and to Makefile.in
This page is out of date. Refresh to see the latest.
View
2  project/VS2010Express/XBMC.vcxproj
@@ -1700,6 +1700,7 @@
<ClCompile Include="..\..\xbmc\utils\UrlOptions.cpp" />
<ClCompile Include="..\..\xbmc\utils\Variant.cpp" />
<ClCompile Include="..\..\xbmc\utils\Weather.cpp" />
+ <ClCompile Include="..\..\xbmc\utils\Environment.cpp" />
<ClCompile Include="..\..\xbmc\utils\XBMCTinyXML.cpp" />
<ClCompile Include="..\..\xbmc\utils\XMLUtils.cpp" />
<ClCompile Include="..\..\xbmc\video\Bookmark.cpp" />
@@ -2504,6 +2505,7 @@
<ClInclude Include="..\..\xbmc\utils\UrlOptions.h" />
<ClInclude Include="..\..\xbmc\utils\Variant.h" />
<ClInclude Include="..\..\xbmc\utils\Weather.h" />
+ <ClInclude Include="..\..\xbmc\utils\Environment.h" />
<ClInclude Include="..\..\xbmc\utils\XBMCTinyXML.h" />
<ClInclude Include="..\..\xbmc\utils\XMLUtils.h" />
<ClInclude Include="..\..\xbmc\video\Bookmark.h" />
View
6 project/VS2010Express/XBMC.vcxproj.filters
@@ -3033,6 +3033,9 @@
<ClCompile Include="..\..\xbmc\cores\dvdplayer\DVDDemuxers\DVDDemuxCDDA.cpp">
<Filter>cores\dvdplayer\DVDDemuxers</Filter>
</ClCompile>
+ <ClCompile Include="..\..\xbmc\utils\Environment.cpp">
+ <Filter>utils</Filter>
+ </ClCompile>
</ItemGroup>
<ItemGroup>
<ClInclude Include="..\..\xbmc\win32\pch.h">
@@ -5939,6 +5942,9 @@
<ClInclude Include="..\..\xbmc\cores\dvdplayer\DVDDemuxers\DVDDemuxCDDA.h">
<Filter>cores\dvdplayer\DVDDemuxers</Filter>
</ClInclude>
+ <ClInclude Include="..\..\xbmc\utils\Environment.h">
+ <Filter>utils</Filter>
+ </ClInclude>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="..\..\xbmc\win32\XBMC_PC.rc">
View
10 xbmc/Application.cpp
@@ -345,6 +345,10 @@
#include "linux/LinuxTimezone.h"
#endif
+#ifdef TARGET_WINDOWS
+#include "utils/Environment.h"
+#endif
+
using namespace std;
using namespace ADDON;
using namespace XFILE;
@@ -703,7 +707,7 @@ bool CApplication::Create()
#elif defined(_LINUX)
setenv("OS","Linux",true);
#elif defined(_WIN32)
- SetEnvironmentVariable("OS","win32");
+ CEnvironment::setenv("OS", "win32");
#endif
g_powerManager.Initialize();
@@ -1174,7 +1178,7 @@ bool CApplication::InitDirectoriesWin32()
CStdString xbmcPath;
CUtil::GetHomePath(xbmcPath);
- SetEnvironmentVariable("XBMC_HOME", xbmcPath.c_str());
+ CEnvironment::setenv("XBMC_HOME", xbmcPath);
CSpecialProtocol::SetXBMCBinPath(xbmcPath);
CSpecialProtocol::SetXBMCPath(xbmcPath);
@@ -1185,7 +1189,7 @@ bool CApplication::InitDirectoriesWin32()
CSpecialProtocol::SetMasterProfilePath(URIUtils::AddFileToFolder(strWin32UserFolder, "userdata"));
CSpecialProtocol::SetTempPath(URIUtils::AddFileToFolder(strWin32UserFolder,"cache"));
- SetEnvironmentVariable("XBMC_PROFILE_USERDATA",CSpecialProtocol::TranslatePath("special://masterprofile/").c_str());
+ CEnvironment::setenv("XBMC_PROFILE_USERDATA", CSpecialProtocol::TranslatePath("special://masterprofile/"));
CreateUserDirs();
View
252 xbmc/utils/Environment.cpp
@@ -0,0 +1,252 @@
+/*
+ * Copyright (C) 2013 Team XBMC
+ * http://www.xbmc.org
+ *
+ * This Program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, 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 XBMC; see the file COPYING. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/**
+ * \file utils\Environment.cpp
+ * \brief Implements CEnvironment class functions.
+ *
+ * Some ideas were inspired by PostgreSQL's pgwin32_putenv function.
+ * Refined, updated, enhanced and modified for XBMC by Karlson2k.
+ */
+
+#include "Environment.h"
+#include <stdlib.h>
+#ifdef TARGET_WINDOWS
+#include <Windows.h>
+#endif
+
+// --------------------- Helper Functions ---------------------
+
+#ifdef TARGET_WINDOWS
+
+std::wstring CEnvironment::win32ConvertUtf8ToW(const std::string &text, bool *resultSuccessful /* = NULL*/)
+{
+ if (text.empty())
+ {
+ if (resultSuccessful != NULL)
+ *resultSuccessful = true;
+ return L"";
+ }
+ if (resultSuccessful != NULL)
+ *resultSuccessful = false;
+
+ int bufSize = MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, text.c_str(), -1, NULL, 0);
+ if (bufSize == 0)
+ return L"";
+ wchar_t *converted = new wchar_t[bufSize];
+ if (MultiByteToWideChar(CP_UTF8, MB_ERR_INVALID_CHARS, text.c_str(), -1, converted, bufSize) != bufSize)
+ {
+ delete[] converted;
+ return L"";
+ }
+
+ std::wstring Wret (converted);
+ delete[] converted;
+
+ if (resultSuccessful != NULL)
+ *resultSuccessful = true;
+ return Wret;
+}
+
+std::string CEnvironment::win32ConvertWToUtf8(const std::wstring &text, bool *resultSuccessful /*= NULL*/)
+{
+ if (text.empty())
+ {
+ if (resultSuccessful != NULL)
+ *resultSuccessful = true;
+ return "";
+ }
+ if (resultSuccessful != NULL)
+ *resultSuccessful = false;
+
+ int bufSize = WideCharToMultiByte(CP_UTF8, MB_ERR_INVALID_CHARS, text.c_str(), -1, NULL, 0, NULL, NULL);
+ if (bufSize == 0)
+ return "";
+ char * converted = new char[bufSize];
+ if (WideCharToMultiByte(CP_UTF8, MB_ERR_INVALID_CHARS, text.c_str(), -1, converted, bufSize, NULL, NULL) != bufSize)
+ {
+ delete[] converted;
+ return "";
+ }
+
+ std::string ret(converted);
+ delete[] converted;
+
+ if (resultSuccessful != NULL)
+ *resultSuccessful = true;
+ return converted;
+}
+
+// --------------------- Internal Function ---------------------
+
+typedef int (_cdecl * wputenvPtr) (const wchar_t *envstring);
+
+/**
+ * \fn int CEnvironment::win32_setenv(const std::wstring &name, const std::wstring &value = L"",
+ * updateAction action = autoDetect)
+ * \brief Internal function used to manipulate with environment variables on win32.
+ *
+ * This function make all dirty work with setting, deleting and modifying environment variables.
+ *
+ * \param name The environment variable name.
+ * \param value (optional) the new value of environment variable.
+ * \param action (optional) the action.
+ * \return Zero on success, 2 if at least one external runtime update failed, 4 if process
+ * environment update failed, 8 if our runtime environment update failed or, in case of
+ * several errors, sum of all errors values; non-zero in case of other errors.
+ */
+int CEnvironment::win32_setenv(const std::string &name, const std::string &value /* = "" */, enum updateAction action /* = autoDetect */)
+{
+ std::wstring Wname (win32ConvertUtf8ToW(name));
+ if (Wname.empty() || name.find('=') != std::wstring::npos)
+ return -1;
+ if ( (action == addOnly || action == addOrUpdateOnly) && value.empty() )
+ return -1;
+ if (action == addOnly && !(getenv(name).empty()) )
+ return 0;
+
+ bool convIsOK;
+ std::wstring Wvalue (win32ConvertUtf8ToW(value,&convIsOK));
+ if (!convIsOK)
+ return -1;
+
+ int retValue = 0;
+ std::wstring EnvString;
+ if (action == deleteVariable)
+ EnvString = Wname + L"=";
+ else
+ EnvString = Wname + L"=" + Wvalue;
+
+ static const wchar_t *modulesList[] =
+ {
+ /*{ L"msvcrt20.dll" }, // Visual C++ 2.0 / 2.1 / 2.2
+ { L"msvcrt40.dll" }, // Visual C++ 4.0 / 4.1 */ // too old and no UNICODE support - ignoring
+ { L"msvcrt.dll" }, // Visual Studio 6.0 / MinGW[-w64]
+ { L"msvcr70.dll" }, // Visual Studio 2002
+ { L"msvcr71.dll" }, // Visual Studio 2003
+ { L"msvcr80.dll" }, // Visual Studio 2005
+ { L"msvcr90.dll" }, // Visual Studio 2008
+ { L"msvcr100.dll" }, // Visual Studio 2010
+#ifdef _DEBUG
+ { L"msvcr100d.dll" },// Visual Studio 2010 (debug)
+#endif
+ { L"msvcr110.dll" }, // Visual Studio 2012
+ { NULL } // Terminating NULL for list
+ };
+
+ // Check all modules each function run, because modules can be loaded/unloaded at runtime
+ for (int i = 0; modulesList[i]; i++)
+ {
+ HMODULE hModule;
+ if (!GetModuleHandleExW(0, modulesList[i], &hModule) || hModule == NULL) // Flag 0 ensures that module will be kept loaded until it'll be freed
+ continue; // Module not loaded
+
+ wputenvPtr wputenvFunc = (wputenvPtr) GetProcAddress(hModule, "_wputenv");
+ if (wputenvFunc != NULL && wputenvFunc(EnvString.c_str()) != 0)
+ retValue |= 2; // At lest one external runtime library Environment update failed
+ FreeLibrary(hModule);
+ }
+
+ // Update process Environment used for current process and for future new child processes
+ if (action == deleteVariable || value.empty())
+ retValue += SetEnvironmentVariableW(Wname.c_str(), NULL) ? 0 : 4; // 4 if failed
+ else
+ retValue += SetEnvironmentVariableW(Wname.c_str(), Wvalue.c_str()) ? 0 : 4; // 4 if failed
+
+ // Finally update our runtime Environment
+ retValue += (::_wputenv(EnvString.c_str()) == 0) ? 0 : 8; // 8 if failed
+
+ return retValue;
+}
+#endif
+
+// --------------------- Main Functions ---------------------
+
+int CEnvironment::setenv(const std::string &name, const std::string &value, int overwrite /*= 1*/)
+{
+#ifdef TARGET_WINDOWS
+ return (win32_setenv(name, value, overwrite ? autoDetect : addOnly)==0) ? 0 : -1;
+#else
+ if (value.empty() && overwrite != 0)
+ return ::unsetenv(name.c_str());
+ return ::setenv(name.c_str(), value.c_str(), overwrite);
+#endif
+}
+
+std::string CEnvironment::getenv(const std::string &name)
+{
+#ifdef TARGET_WINDOWS
+ std::wstring Wname (win32ConvertUtf8ToW(name));
+ if (Wname.empty())
+ return "";
+
+ std::wstring Wvalue (::_wgetenv(Wname.c_str()));
+ if (!Wvalue.empty())
+ return win32ConvertWToUtf8(Wvalue);
+
+ // Not found in Environment of runtime library
+ // Try Environment of process as fallback
+ int varSize = GetEnvironmentVariableW(Wname.c_str(), NULL, 0);
+ if (varSize == 0)
+ return ""; // Not found
+ wchar_t * valBuf = new wchar_t[varSize];
+ if (GetEnvironmentVariableW(Wname.c_str(), valBuf, varSize) != varSize-1)
+ {
+ delete[] valBuf;
+ return "";
+ }
+ Wvalue = valBuf;
+ delete[] valBuf;
+
+ return win32ConvertWToUtf8(Wvalue);
+#else
+ return ::getenv(name.c_str());
+#endif
+}
+
+int CEnvironment::unsetenv(const std::string &name)
+{
+#ifdef TARGET_WINDOWS
+ return (win32_setenv(name, "", deleteVariable)) == 0 ? 0 : -1;
+#else
+ return ::unsetenv(name.c_str());
+#endif
+}
+
+int CEnvironment::putenv(const std::string &envstring)
+{
+ if (envstring.empty())
+ return 0;
+ int pos = envstring.find('=');
+ if (pos == 0) // '=' is the first character
+ return -1;
+ if (pos == std::string::npos)
+ return unsetenv(envstring);
+ if (pos == envstring.length()-1) // '=' is in last position
+ {
+ std::string name(envstring);
+ name.erase(name.length()-1, 1);
+ return unsetenv(name);
+ }
+ std::string name(envstring, 0, pos), value(envstring, pos+1);
+
+ return setenv(name, value);
+}
+
View
106 xbmc/utils/Environment.h
@@ -0,0 +1,106 @@
+#pragma once
+#ifndef XBMC_SETENV_H
+#define XBMC_SETENV_H
+/*
+ * Copyright (C) 2013 Team XBMC
+ * http://www.xbmc.org
+ *
+ * This Program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, 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 XBMC; see the file COPYING. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/**
+ * \file utils\Environment.h
+ * \brief Declares CEnvironment class for platform-independent environment variables manipulations.
+ *
+ */
+#include <string>
+
+/**
+ * @class CEnvironment
+ *
+ * @brief Platform-independent environment variables manipulations.
+ *
+ * Provide analog for POSIX functions:
+ * + setenv
+ * + unsetenv
+ * + putenv
+ * + getenv
+ *
+ * You can generally use the functions as you would normally in POSIX-style.
+ * The differences below are just to make things more convenient through use of std::string (2,3),
+ * and to also allow the Win32-style of unsetting variables (4,5) if wanted.
+ * 1. CEnvironment::setenv parameter 'overwrite' is optional, set by default to 1 (allow overwrite).
+ * 2. CEnvironment::putenv uses copy of provided string (rather than string itself) to change environment,
+ * so you can free parameter variable right after call of function.
+ * 3. CEnvironment::getenv returns a copy of environment variable value instead of pointer to value.
+ * 4. CEnvironment::setenv can be used to unset variables. Just pass empty string for 'value' parameter.
+ * 5. CEnvironment::putenv can be used to unset variables. Set parameter to 'var=' (Windows style) or
+ * just 'var' (POSIX style), and 'var' will be unset.
+ *
+ * All 'std::string' types are supposed to be in UTF-8 encoding.
+ * All functions work on all platforms. Special care is taken on Windows platform where Environment is changed for process itself,
+ * for process runtime library and for all runtime libraries (MSVCRT) loaded by third-party modules.
+ * Functions internally make all necessary UTF-8 <-> wide conversions.*
+ */
+
+class CEnvironment
+{
+public:
+ /**
+ * \fn static int CEnvironment::setenv(const std::string &name, const std::string &value,
+ * int overwrite = 1);
+ * \brief Sets or unsets environment variable.
+ * \param name The environment variable name to add/modify/delete.
+ * \param value The environment variable new value. If set to empty string, variable will be
+ * deleted from the environment.
+ * \param overwrite (optional) If set to non-zero, existing variable will be overwritten. If set to zero and
+ * variable is already present, then variable will be unchanged and function returns success.
+ * \return Zero on success, non-zero on error.
+ */
+ static int setenv(const std::string &name, const std::string &value, int overwrite = 1);
+ /**
+ * \fn static int CEnvironment::unsetenv(const std::string &name);
+ * \brief Deletes environment variable.
+ * \param name The environment variable name to delete.
+ * \return Zero on success, non-zero on error.
+ */
+ static int unsetenv(const std::string &name);
+
+ /**
+ * \fn static int CEnvironment:putenv(const std::string &envstring);
+ * \brief Adds/modifies/deletes environment variable.
+ * \param envstring The variable-value string in form 'var=value'. If set to 'var=' or 'var', then variable
+ * will be deleted from the environment.
+ * \return Zero on success, non-zero on error.
+ */
+ static int putenv(const std::string &envstring);
+ /**
+ * \fn static std::string CEnvironment::getenv(const std::string &name);
+ * \brief Gets value of environment variable in UTF-8 encoding.
+ * \param name The name of environment variable.
+ * \return Copy of of environment variable value or empty string if variable in not present in environment.
+ * \sa xbmc_getenvUtf8, xbmc_getenvW
+ */
+ static std::string getenv(const std::string &name);
+#ifdef TARGET_WINDOWS
+private:
+ static std::wstring win32ConvertUtf8ToW(const std::string &text, bool *resultSuccessful = NULL);
+ static std::string win32ConvertWToUtf8(const std::wstring &text, bool *resultSuccessful = NULL);
+ enum updateAction:int {addOrUpdateOnly = -2, deleteVariable = -1, addOnly = 0, autoDetect = 1};
+ static int win32_setenv(const std::string &name, const std::string &value = "", updateAction action = autoDetect);
+#endif // TARGET_WINDOWS
+};
+#endif
View
1  xbmc/utils/Makefile.in
@@ -16,6 +16,7 @@ SRCS += DownloadQueue.cpp
SRCS += DownloadQueueManager.cpp
SRCS += EndianSwap.cpp
SRCS += EdenVideoArtUpdater.cpp
+SRCS += Environment.cpp
SRCS += Fanart.cpp
SRCS += fastmemcpy.c
SRCS += fastmemcpy-arm.S
Something went wrong with that request. Please try again.