In a DLL compiled from C++ with MS Visual Studio 2003, I'm generating a file path by getting a string from SHGetFolderPath and appending a file name, then trying to open that file for output.
I have some globals, one function to setup the file path, and another function to open the file, all shown below.
The Logger
class declaration isn't shown, but it has a member declared as ofstream *ofs
which is the file object created in Logger
's constructor.
In constructorBase()
, if I call initLogFN()
to setup the file path, ofs->open
fails.
However, if I comment out the initLogFN()
call and uncomment the line that sets logPath
to a hard-coded string literal, ofs->open
succeeds.
When debugging in either case, Visual Studio correctly shows logPath
as having the value "C:\Users\sleis\AppData\Roaming\Address-IT.log" before ofs->open
is called.
However in the failing case, after the ofs->open
call, logPath
's value changes to six characters that seem random but are the same every time I run my test app.
Why do the two cases behave differently, and how can I fix the failing case?
#if defined (WIN32)
const char dirSep[]="\\";
#else
const char dirSep[]="/";
#endif
const char logFN[]="Address-IT.log";
char *logPath=0;
bool initLogFN()
{
if (logPath!=0) return false;
#if defined (_DEBUG)
#if defined (WIN32)
char path[MAX_PATH];
HRESULT result=SHGetFolderPath(NULL,CSIDL_APPDATA,NULL,SHGFP_TYPE_CURRENT,path);
if (result!=S_OK) return false;
strcat(path,dirSep);
strcat(path,logFN);
logPath=path;
return true;
#endif
#endif
return false;
}
void Logger::constructorBase()
{
if (!initLogFN()) return;
//logPath="C:\\Users\\sleis\\AppData\\Roaming\\Address-IT.log";
ofs->open(logPath,ios_base::out | ios_base::app);
if ((ofs->rdstate() & std::ifstream::failbit)!=0) {
std::cout << std::endl << "Error opening log file.";
} else if (ofs->is_open()) {
std::cout << std::endl << "Log file opened successfully.";
}
}
The array path
is an automatic variable local to initLogFN
. When you set logPath=path
, it causes the logPath
variable to point into this array. But when the function returns, that array goes out of scope, which means whatever memory it occupied may be subsequently overwritten. Now path
points to garbage.
Never return a pointer to a local variable.
You essentially have three options for returning a C-style string from a function:
Have the function dynamically allocate the memory for the string, using malloc
or (in C++) new
. If you do this, then the caller will have to eventually call free
or delete
to avoid memory leaks.
Pass the char*
as a parameter to the function. The caller should pre-allocate the memory for the string and then pass the pointer into the function. The function writes into the location the pointer points to.
Use a global variable, and return a pointer to that.
Of these approaches, #2 is the most common. The C and C++ standard libraries take this approach (e.g. fgets
), and generally require another parameter indicating the size of the buffer passed in, so the function knows not to write past the end of allocated memory.
The disadvantage of approach #3 is that it is not reentrant. A few old Unix library functions take approach #3 (e.g. ttyname
), but reentrant versions that take approach #2 were later introduced (e.g., ttyname_r
).