1

I try to catch up with C++ and ran in some problems for which I cannot find a solution. I'm working with a class which is a simple version of std::string, this class is for 'testing/learning purposes only'. Who can help me with some advise on what I'm doing wrong.

The problem in short:

I created a class Str with a copy constructor and a destructor. Direct initialisation gives no problem.

Str a = "Hello";  // OK.

Assignment is failing;

Str a;
a = "Hello";  // Failing.

A disassembly of the code and an identification on what goes wrong (according to me) is available below.

Note: the version with a = Str("Hello"); shows the same effect.

I know what goes wrong but I do not know how to correct it. In the first case

Str a = "Hello"; 
// The constructor Str::Str(const char* value) is used. No problem.

In the second case

Str a;
// The default constructor Str::Str() is used. No problem.

a = "Hello";
// 001D1557: The constructor Str::Str(const char* value) is used creating instance a'. No problem.
// 001D155C: Instance a' is copied over a. No problem yet.
// 001D157D: The desctuctor for a' is called, which in the process destoys the char* used by a, corrupting it.

What am I doing wrong here? Am I missing some point on the C++ semantics on how to assign a value?

Here is the actual code that I use.

#pragma once

namespace Steenveld {
namespace Base {

class Str {
public:
    Str();
    Str(const Str& value);
    Str(const Str* value);
    Str(const char* value);
    ~Str();

    inline const size_t& Length() const { return length; };
    inline const char* Value() const { return buffer; };

private:
    char* buffer;
    size_t length;
    size_t size;

    void Init(void);
    void SetBuffer(size_t size);
};

} // namespace Base
} // namespace Steenveld

And its implementation:

#include "stdafx.h"

namespace Steenveld {
namespace Base {

Str::Str() {
    Init();
}

Str::Str(const Str& value) {
    Init();
    length = value.length;
    SetBuffer(length+1);
    strcpy_s(buffer, size, value.buffer);
}

Str::Str(const Str* value) {
    Init();
    length = value->length;
    SetBuffer(length+1);
    strcpy_s(buffer, size, value->buffer);
}

Str::Str(const char* value) {
    Init();
    if (value != nullptr) {
        length = strlen(value);
        SetBuffer(length+1);
        strcpy_s(buffer, size, value);
    }
}

Str::~Str() {
    if (buffer != nullptr) {
        free(buffer);
        buffer = nullptr;
        size = 0;
        length = 0;
    }
}

void Str::Init(void) {
    buffer = nullptr;
    length = 0;
    size = 0;
}

void Str::SetBuffer(size_t size) {
    if (size <= 0) return;
    if (buffer == nullptr) {
        buffer = (char*)malloc(size);
        this->size = size;
    } else if (this->size < size) {
        buffer = (char*)realloc(buffer, size);
        this->size = size;
    }
    buffer[size-1] = '\0';
}

} // namespace Base
} // namespace Steenveld

VS2010 header file

// stdafx.h : include file for standard system include files,
// or project specific include files that are used frequently, but
// are changed infrequently

#pragma once
#include "targetver.h"
#include <iostream>
#include <tchar.h>
#include "Str.h"

And the actual program.

#include "stdafx.h"

using namespace Steenveld::Base;

int _tmain(int argc, _TCHAR* argv[])
{
    Str a;
    a = "Hell";

    std::cout << a.Value() << "\r\n";
    char n;
    std::cin.get(n);
    return 0;
}

The disassembly of the program part.

--- e:\andre\ontwikkeling\libconsole\consolenative\program.cpp -----------------
// ConsoleNative.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

using namespace Steenveld::Base;

int _tmain(int argc, _TCHAR* argv[])
{
001D1500  push        ebp  
001D1501  mov         ebp,esp  
001D1503  push        0FFFFFFFFh  
001D1505  push        offset __ehhandler$_wmain (1D5718h)  
001D150A  mov         eax,dword ptr fs:[00000000h]  
001D1510  push        eax  
001D1511  sub         esp,100h  
001D1517  push        ebx  
001D1518  push        esi  
001D1519  push        edi  
001D151A  lea         edi,[ebp-10Ch]  
001D1520  mov         ecx,40h  
001D1525  mov         eax,0CCCCCCCCh  
001D152A  rep stos    dword ptr es:[edi]  
001D152C  mov         eax,dword ptr [___security_cookie (1D9000h)]  
001D1531  xor         eax,ebp  
001D1533  push        eax  
001D1534  lea         eax,[ebp-0Ch]  
001D1537  mov         dword ptr fs:[00000000h],eax  
    Str a;
001D153D  lea         ecx,[ebp-1Ch]  
001D1540  call        Steenveld::Base::Str::Str (1D100Ah)  
001D1545  mov         dword ptr [ebp-4],0  
    a = "Hell";
001D154C  push        offset string "Hell" (1D7740h)  
001D1551  lea         ecx,[ebp-108h]  
001D1557  call        Steenveld::Base::Str::Str (1D1037h)  
001D155C  mov         eax,dword ptr [ebp-108h]  
001D1562  mov         dword ptr [ebp-1Ch],eax  
001D1565  mov         ecx,dword ptr [ebp-104h]  
001D156B  mov         dword ptr [ebp-18h],ecx  
001D156E  mov         edx,dword ptr [ebp-100h]  
001D1574  mov         dword ptr [ebp-14h],edx  
001D1577  lea         ecx,[ebp-108h]  
001D157D  call        Steenveld::Base::Str::~Str (1D1113h)  

    std::cout << a.Value() << "\r\n";
001D1582  push        offset string "\r\n" (1D773Ch)  
001D1587  lea         ecx,[ebp-1Ch]  
001D158A  call        Steenveld::Base::Str::Value (1D1104h)  
001D158F  push        eax  
001D1590  mov         eax,dword ptr [__imp_std::cout (1DA33Ch)]  
001D1595  push        eax  
001D1596  call        std::operator<<<std::char_traits<char> > (1D116Dh)  
001D159B  add         esp,8  
001D159E  push        eax  
001D159F  call        std::operator<<<std::char_traits<char> > (1D116Dh)  
001D15A4  add         esp,8  
    char n;
    std::cin.get(n);
001D15A7  mov         esi,esp  
001D15A9  lea         eax,[ebp-25h]  
001D15AC  push        eax  
001D15AD  mov         ecx,dword ptr [__imp_std::cin (1DA340h)]  
001D15B3  call        dword ptr [__imp_std::basic_istream<char,std::char_traits<char> >::get (1DA330h)]  
001D15B9  cmp         esi,esp  
001D15BB  call        @ILT+430(__RTC_CheckEsp) (1D11B3h)  
    return 0;
001D15C0  mov         dword ptr [ebp-0F4h],0  
001D15CA  mov         dword ptr [ebp-4],0FFFFFFFFh  
001D15D1  lea         ecx,[ebp-1Ch]  
001D15D4  call        Steenveld::Base::Str::~Str (1D1113h)  
001D15D9  mov         eax,dword ptr [ebp-0F4h]  
}
001D15DF  push        edx  
001D15E0  mov         ecx,ebp  
001D15E2  push        eax  
001D15E3  lea         edx,[ (1D1610h)]  
001D15E9  call        @ILT+160(@_RTC_CheckStackVars@8) (1D10A5h)  
001D15EE  pop         eax  
001D15EF  pop         edx  
001D15F0  mov         ecx,dword ptr [ebp-0Ch]  
001D15F3  mov         dword ptr fs:[0],ecx  
001D15FA  pop         ecx  
001D15FB  pop         edi  
001D15FC  pop         esi  
001D15FD  pop         ebx  
001D15FE  add         esp,10Ch  
001D1604  cmp         ebp,esp  
001D1606  call        @ILT+430(__RTC_CheckEsp) (1D11B3h)  
001D160B  mov         esp,ebp  
001D160D  pop         ebp  
001D160E  ret  
001D160F  nop  
001D1610  db          02h  
001D1611  db          00h  
001D1612  db          00h  
001D1613  db          00h  
001D1614  db          18h  
001D1615  db          16h  
001D1616  db          1dh  
001D1617  db          00h  
001D1618  db          e4h  
001D1619  db          ffh  
001D161A  db          ffh  
001D161B  db          ffh  
001D161C  db          0ch  
001D161D  db          00h  
001D161E  db          00h  
001D161F  db          00h  
001D1620  db          32h  
001D1621  db          16h  
001D1622  db          1dh  
001D1623  db          00h  
001D1624  db          dbh  
001D1625  db          ffh  
001D1626  db          ffh  
001D1627  db          ffh  
001D1628  db          01h  
001D1629  db          00h  
001D162A  db          00h  
001D162B  db          00h  
001D162C  db          30h  
001D162D  db          16h  
001D162E  db          1dh  
001D162F  db          00h  
001D1630  db          6eh  
001D1631  db          00h  
001D1632  db          61h  
001D1633  db          00h
PapaAtHome
  • 575
  • 8
  • 25
  • 3
    I thought you'd need `Str& operator=(const Str& value)` for that. – barak manos Dec 21 '14 at 11:40
  • @barakmanos: No `operator=` needed, it has a `char*` ctor available, then the default assignment operator will do a simple member-by-member copy (which will kill the string in the destructor). Example: `#include class A { public: A() : s_(nullptr) { std::cout << "default ctor called\n"; } A(char* s) : s_(s) { std::cout << "A(char*) ctor called\n"; } ~A() { std::cout << "Would delete: " << static_cast(s_) << "\n"; } private: char *s_; }; int main() { A a; a = "foo"; } ` – frasnian Dec 21 '14 at 12:02
  • @πάντα ῥεῖ: Nice to know that it is already answered before. Where can I find the question and answer. I have tried to look for it. (*before* I posted my question.) – PapaAtHome Dec 21 '14 at 12:04
  • Sorry about the unformatted code in my comment - I misread the minimarkdown help for what I could do there. – frasnian Dec 21 '14 at 12:09
  • 1
    @PapaAtHome _"Where can I find the question and answer."_ The link is just shown above your question. – πάντα ῥεῖ Dec 21 '14 at 12:13
  • @frasnian: Your code shows the same behaviour. The difference is that you do not release the space pointed to by s_ (potential memory leak?). In my version I manage the space used in Str myself and the destructor must release it, that is causing me problems. – PapaAtHome Dec 21 '14 at 12:30
  • @PapaAtHome: my code was to illustrate the point, that is *why* it shows the same behavior. It was in response to the implied question in another poster's comment about not needing an assignment operator for your problem to occur. – frasnian Dec 21 '14 at 12:33
  • 1
    @frasnian: Thanks for the example, combined with 'What is The Rule of Three' is learns me where I went wrong. Using a chain of constructors, e.g. using : s_(nullptr), is the recommended way as I see now. I need to do some rethinking and fooling around with code examples to fully appreciate that style. – PapaAtHome Dec 21 '14 at 13:01
  • I consider this question closed. (But don't know how to mark it as such.) – PapaAtHome Dec 21 '14 at 13:02
  • @PapaAtHome _"I consider this question closed."_ Don't worry, a marked dupe is considered closed. You don't need to delete it, it may be helpful for others to find your question, and the duplicate link. I've upvoted your question, since you obviously did some efforts, to dig on the reasons for your problem. – πάντα ῥεῖ Dec 21 '14 at 15:29

0 Answers0