legacy code and exception handling After some 'digging' into the dark corners of legacy code I've found class, which handles INI files. It does reading and writing to the file, but I haven't found any exception handling logic. What kind of potential problems, if any, can you detect and what is your suggestion/advise for improving the code, based on these three C++ code snippets:

Constructor

IniValue::IniValue(const char* FileName, const char* Section, const char* Entry, int sz, PersistMediumEnum medium_, int iniprofileId_)
:filename(NULL)
,section(new char[strlen(Section)+1])
,entry(new char[strlen(Entry)+1])
,buffer(new char[sz])
,bufsiz(sz)
{
  if (FileName == NULL) {
    FileName = "somefile.ini";
  }

  // 'GetFullInifileName' calls windows functions 'GetEnvironmentVariable', 'CreateDirectory', 'CopyFile', 'DeleteFile'
  std::string fullname = GetFullInifileName(FileName);

  filename = new char[fullname.length()+1]; strcpy(filename, fullname.c_str());

    strcpy(section, Section);
    strcpy(entry, Entry);
    buffer[0] = 0;
}


Some function, which reads values from the file:

void IniValue::TheGetProfileString() {
    char* defstr = new char[bufsiz];
    strncpy(defstr, buffer, bufsiz);

  IniPersistIF* inf = IniPersistIF::get(medium);

  //calls windows 'GetPrivateProfileString'
  inf-&gt;Read(section, entry, defstr, buffer, bufsiz, filename, iniprofileId);
  delete [] defstr;
}


Destructor

IniValue::~IniValue() {
  IniValue* thys = this;

  assert(section[0] != '\0');
  assert(filename[0] != '\0');

  IniPersistIF* inf = IniPersistIF::get(medium);
  assert(inf != NULL);

  assert(section[0] != '\0');
  assert(filename[0] != '\0');

  // Calls Windows 'WritePrivateProfileString' function
  inf-&gt;Write(section, entry, buffer, filename, iniprofileId, defaultval, bModified); 

  delete [] buffer;
  delete [] entry;
  delete [] section;
  delete [] filename;
}
This was posted just a short while ago on StackOverflow.com

Firstly, if this is legacy code that has existed in production for a long time, then even if you can make it semantically better, you have to really weigh up the gains against the losses. Changing code is a cost, can introduce bugs even when the new code appears to be a lot better, and has to be justified. That the existing code uses a lot of arrays does not mean it has any bugs, nor would the "improvements" give you better performance in any way.

However, on the basis of giving this code a review I will do so.

Basically, start off by getting rid of all the arrays and calls to new[] and delete[] and use std::string (which is used there in one place so it is obviously known to the programmer).

Ensure your IniPersistInf class is const-correct so takes if it takes a string that it only reads, it uses const char * or const std::string &amp; rather than char *.

buffer in IniValue appears to be for writing into, so for this one you might use std::vector&lt;char&gt;. In such a case, to get the pointer out of it, use &amp;buffer[0] when you pass it to the IniPersistIf methods.