Bizarre error with Borland C++ and GetUserName

Liam OHagan

So I've been tasked with maintaining some old code built with Borland C++ builder 6 (which was old in 2007 when the code was originally written). I can't change to a new compiler for regulatory reasons, so I'm stuck with Borland (which has a terrible UI and is exceedingly difficult to navigate, but that's another story) There's 14.9 million lines of code for a single, simple, 1.7MB executable, (it was developed by a company in France, who ran out of money and subcontracted out finishing the program to another company in Germany, who ran out of money and sub-subcontracted out the program to another company in Russia, but that's also another story...) I get tasked with changing a report to show the currently logged in windows user, rather than a manually entered username. Google gave me some sample code using a handy function called GetUserName, so I give that a go, and it returns the username, excellent! Then I go to use the returned value in the code, and all hell breaks loose! Here's the code:

char* user = "";
long* size;
GetUserName(user, size);
AnsiString UserName (user);
SetInfoAsString("ManuallyEnteredUserName", UserName);

Where SetInfoAsString is defined as:

void BaseOperation::SetInfoAsString(AnsiString Key, AnsiString Id)
{
Context::GetInstance().SetInfo(Key, Id);
}

And where SetInfo is defined as :

void Context::SetInfo(const AnsiString& key, const WideString& value)
{
tPMap::iterator it = m_PMap.find(key);
if (it == m_PMap.end())
throw ProgramException(OPERATION_UNIT_EXCEPTION, "Program key " + key + " not found");

it->second = value;

}

The ProgramException was thrown, saying the key couldn't be found. Turns out, between the call to SetInfoAsString("ManuallyEnteredUserName", UserName); and the Context::GetInstance().SetInfo(Key, Id);, Key had somehow become corrupted! For example, say my username is 'liamo', one would expect that calling SetInfoAsString("ManuallyEnteredUserName", UserName); with UserName as 'liamo' would result in a call to Context::GetInstance().SetInfo(Key, Id); with Key as 'ManuallyEnteredUserName' and Id as 'liamo', but what we were seeing was Key as 'iamo' and Id as 'liamo' :omg: Eventually, I narrowed it down to this code:

char* user = "";
long* size;
GetUserName(user, size);
AnsiString UserName (user);

Changing it to the following resolved the problem: <

Super Lloyd

Liam O'Hagan wrote:

Eventually, I narrowed it down to this code: char* user = ""; long* size; GetUserName(user, size); AnsiString UserName (user); Changing it to the following resolved the problem: char user[30]; unsigned long userSize = sizeof(user); GetUserName(user, &userSize); AnsiString UserName(user);

hey, that's obvious, without doing any C++ fu! GetUserName(char* output, unsigned* len) len would be update by the function, and needs to points to an allocated memory area! When you write

Liam O'Hagan wrote:

long* size; GetUserName(user, size);

The GetUserName() method write to the area pointed by size which, being unitialized, is a random area of your memory!!!!! :omg:

A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.

Liam OHagan

That'll teach us to trust random code from google huh! :doh:

I have no blog...

Nemanja Trifunovic

FWIIW, your current solution is still not good:

char user[30];

If the user name is longer than 30 characters you are in trouble. According to the documentation[^], the maximum length for the user name buffer is UNLEN + 1.

utf8-cpp

aritosteles

Hi! The right (and only) way to code this is following: #include "Lmcons.h" //if needed char szUserName[UNLEN + 1]; DWORD nSize= UNLEN + 1; GetUserName( szUserName,&nSize); And it's the same for a number of windows functions like GetModuleFileName(), GetWindowText(), and so many others. Aritosteles

Nemanja Trifunovic

That is of course a correct way to do it, but I wouldn't say the only way :) For instance:

vector<char> user_name(UNLEN+1);
DWORD size(user_name.size());

GetUserName(&user_name[0], &size);

utf8-cpp

Liam OHagan

This is using borland C++ builder, and UNLEN is not defined unfortunately, so we went with 256.

I have no blog...

sashoalm

My school teacher used to make the same mistake, because we had just recently switched to Borland C from Turbo Pascal. She would write on the blackboard programs like that:

char* text;
scanf("%s", text);

// do stuff

The computers were on DOS at that time, and because there are no access violations in DOS, it worked most of the time - unless you happened to overwrite something important. Now in hindsight, that explains why we had to reboot the computers so often.

There is sufficient light for those who desire to see, and there is sufficient darkness for those of a contrary disposition. Blaise Pascal

Dimitris Vasiliadis

I think a generally safer way is to ask for the required size, allocate the memory, and then provide the buffer to store the value. e.g.

char* name = NULL;
DWORD namesize = 0;
//Ask for required size
GetUserName(NULL, &namesize);
name = (char*)malloc(namesize+1);
GetUserName(name, &namesize);
//Do whatever...
free(name);

...Plug & Pray... X|