#266 Problem with environment variables in b140

[Michael Kairys]

In my user environment I have HOME = %HOMEDRIVE%%HOMEPATH%, where HOMEDRIVE = D: and HOMEPATH = \users\Michael (set by Windows via my account properties). In a Bash window if I echo $HOME I get /users/Michael. However in the malfunctioning console window if I echo $HOME I get /users/Michael/%HOMEDRIVE%%HOMEPATH%. This does not happen with b139.

Similarly, Lucian Ioan (xtndd) reports:

I can confirm that in b140 shell variables are lost (in b139 it's ok).

To reproduce it:
- start cmd.exe
- in the cmd window: set a variable (e.g. set myvar=abc)
- in the cmd window: start console
- in the console window: display the variable (e.g. echo %myvar%)

In b139 you'll get "abc".
In b140 you'll get "%myvar%".

[Marko Bozikovic]

Ok, I think I have fixed this, it will be available in the next build

[Kirill]

Marko,

I think you fixed only the simpler one (the one from Lucian Ioan) in b141. I don't think the one from Michael Kairys is fixed because of the following scenario:
- start Console;
- verify that you don't have HOME by
echo %home%
- add an environment variable HOME=%HOMEDRIVE%%HOMEPATH% in My Computer's properties;
- verify that you still don't have HOME in your current tab by
echo %home%
- open a new tab;

At this moment, my echo %home% displays "%HOMEDRIVE%%HOMEPATH%" instead of usual "c:\users\me".

It looks like Windows does some magic in setting up an environment block, but not when CreateProcess is used (any way, my quick search through various CreateProcess' structure did not reveal any obvious flag to do the same magic). So, I tried to reproduce the magic and you can find the result at the end of this message. Please, excuse me for such a messy implementation, but I'm not too familiar with Boost which may or may not be helpful to deal with NULL-terminated array of NULL-terminated strings.

However... while I was testing the function, I came to realization that the idea to watch for environment changes is great, but the design is not as straightforward as it seems. Try this:
- start debugging of Console from VS;
- execute "set" to see the environment;
- change environment in My Computer's properties;
- open new tab;
- execute "set" to see the environment.

I do understand that opening a new tab is kind of Start -> Run operation (so it does not inherit the environment), but I would expect opening a new tab and starting a new instance of Console [from a Console's tab] behave in a similar way. Moreover, if you ever come to implementing single-instance setting, it may become even more confusing.

So, my proposal: either make this new behaviour configurable or revert back to the original one [simple inheritance].
Additional argument in favour of the original behaviour: when you say the Console watches the environment changes, less technical-savvy users will ask you "I changed My Computer's properties by adding a variable, but echo %variable% does not work. Why?" and it'll be kind of difficult to explain that it works only for new tabs and not even for new Console instances.

Hope the above makes some sense. And here is the promised new version of

void ConsoleHandler::UpdateEnvironmentBlock()
{
void* pEnvironment = NULL;
HANDLE hProcessToken = NULL;

::OpenProcessToken(::GetCurrentProcess(), TOKEN_ALL_ACCESS, &hProcessToken);
::CreateEnvironmentBlock(&pEnvironment, hProcessToken, FALSE);

DWORD dwSize = 0;
void *pExpanded = malloc(_MAX_ENV * sizeof(TCHAR));
TCHAR *pszNew = (TCHAR *)pEnvironment, *pszExpanded = (TCHAR *)pExpanded;
while (*pszNew)
{
if (!::ExpandEnvironmentStringsForUser(hProcessToken, pszNew, pszExpanded, _MAX_ENV))
{
// if somehow it fails, just copy unexpanded string over
_tcscpy_s(pszExpanded, dwSize + _MAX_ENV, pszNew);
}

DWORD dwNewSize = _tcslen(pszExpanded);
dwSize += dwNewSize + 1;

// reallocate buffer to be big enough for _MAX_ENV chars next time
pExpanded = realloc(pExpanded, (dwSize + _MAX_ENV) * sizeof(TCHAR));
pszExpanded = ((TCHAR *)pExpanded) + dwSize;

pszNew += _tcslen(pszNew) + 1;
}

// put the closing (second) NULL terminator at the end of the block
*pszExpanded = 0;
pExpanded = realloc(pExpanded, (dwSize + 1) * sizeof(TCHAR));

::CloseHandle(hProcessToken);
::DestroyEnvironmentBlock(pEnvironment);

s_environmentBlock.reset(pExpanded, free);
}

--
Kirill.

[Marko Bozikovic]

Kirill,

I agree: this behaviour should be configurable...

This is also strange: if I create a new variable, say 'TEST' and set its value to %HOME%, it show expanded in the new shell. If I set 'TEST' to %HOMEDRIVE%%HOMEPATH%, it is not expanded.

I will add environment expansion code, like you suggested...

[Marko Bozikovic]

Michael,

Some environment variables, like HOMEDRIVE and HOMEPATH are handled differently in Windows. Take a look at this link: http://support.microsoft.com/kb/101507

User's HOME variable has a default value. That can be overridden using User manager and environment variables applet. HOMEDRIVE and HOMEPATH are set according to the value of HOME variable. I suspect that's why HOME=%HOMEDRIVE%%HOMEPATH% leads to undefined behavior...