0

I have same code:

.hpp file:

class CConsoleModel
{
char* ParametersBuffer;

...

public:
CConsoleModel() ;  // - basic constructor;
~CConsoleModel() ; // -basic destructor
char *DeterminationParameter(std::string _command, int _parametersize);
...
};

.cpp file:

char *CConsoleModel::DeterminationParameter(std::string _command, int _parametersize)
{
  ParametersBuffer = new char[_parametersize];
  unsigned int HexValue;
 _command = _command.substr(_command.length() - (_parametersize*2),(_parametersize*2));
  //do conversion of the string to the required dimension (_parametrsize):
  for (int i(0); i<_parametersize;i++)
  {
    std::stringstream CommandSteam;
    CommandSteam<< std::hex <<_command[2*i];
    CommandSteam<< std::hex <<_command[2*i +1];
    CommandSteam >> std::hex >> HexValue;
    ParametersBuffer[i] = static_cast<char> (HexValue);
  }
  return  ParametersBuffer;
}

Program build, but crash when run.

If I change ParametersBuffer = new char[_parametersize]

to char* ParametersBuffer = new char[_parametersize]

everything works. How can I fix this problem?

Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • 1
    I suspect that the problem is in the `...`s. Post the smallest code example you can come up with that compiles and runs and shows the problem. – Pete Becker Sep 04 '13 at 15:35
  • 2
    There's not enough code to be sure, but you're probably forgetting the [Rule of Three](http://stackoverflow.com/questions/4172722), copying the `CConsoleModel`, and deleting the same buffer twice. Why not use `std::string` or `std::vector` rather than messing around with `new`? – Mike Seymour Sep 04 '13 at 15:36
  • My guess is that you delete the ParametersBuffer multiple times, or use it after deletion. Do you delete ParametersBuffer in the destructor (`~CConsoleModel`)? – zennehoy Sep 04 '13 at 15:36
  • Yes, I use delete[] ParametersBuffer in ~CConsoleModel – user2746837 Sep 04 '13 at 15:40
  • @user2746837: Have you also implemented or deleted the copy constructor and copy-assignment operator, per the [Rule of Three](http://stackoverflow.com/questions/4172722)? If not, the class is a crash waiting to happen. – Mike Seymour Sep 04 '13 at 15:57
  • @BenVoigt: yeah, my mistake - would have seen my mistake much quicker if the code had a scope identifier on the variable (e.g. `m_ParametersBuffer`). Multiple `delete`s seem to be the issue but hard to tell without seeing a bit more code. – Skizz Sep 04 '13 at 15:59

2 Answers2

2

We strongly recommend to use std::vector instead of manual memory allocation.

class CConsoleModel
{
    std::vector<char> ParametersBuffer;

and

ParametersBuffer.resize(_parametersize);

...

return &ParametersBuffer[0];

BTW

std::stringstream CommandSteam;
    CommandSteam<< std::hex <<_command[2*i];
    CommandSteam<< std::hex <<_command[2*i +1];
    CommandSteam >> std::hex >> HexValue;

is horrible and won't work when you have single digit values. Try

HexValue = (_command[2*i] << 8) | _command[2*i+1];
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
0

My psychic debugging skills tell me that you're missing either the copy constructor or copy assignment operator (or that your destructor doesn't properly clean up the buffer).

If you just use std::string your problems with this will be solved.

Mark B
  • 95,107
  • 10
  • 109
  • 188