"this" gets resetted C++

Question

I am trying to make a class like string(for learning purposes) and i have the following files

var.cpp:

#include "var.hpp"


var::var(){}

var::var(const char* v) {
    (*this) = v;
}

var var::operator=(const char*& v) {
    if(string_var) {
        free((void*)string_var);
        string_var = NULL;
    }
    if(!v) return (*this);

    string_var = strdup(v);
    return (*this);
}

var::~var() {
    if(string_var && *string_var) free((void*)string_var);
}

var.hpp:

#include <iostream>
#include <cstring>
#include <cstdlib>


class var {
private:
    const char* string_var = NULL;
public:
    var();
    var(const char* v);
    var operator=(const char*&);
    ~var();
};

test.cpp:

#include "var.hpp"

int main() {
    var v1 = "test";
}

it compiles without any errors.

with gdb i ran the compiled program and i found that when it goes from the constructor to the operator=, the operator= does its job correctly but when it returns i check the this(like: p *this) and the string_var is "".

I am still learning so please help me understand why and how to fix it.

EDIT

After some debugging i found that the destructor gets called inside the constructor; at least that's what i understand from the following output:

Breakpoint 1, var::~var (this=0x7fffffffde60, __in_chrg=<optimized out>) at var.cpp:21
21  var::~var() {
(gdb) back
#0  var::~var (this=0x7fffffffde60, __in_chrg=<optimized out>) at var.cpp:21
#1  0x00005555555552ef in var::var (this=0x7fffffffde80, v=0x555555556005 "ok") at var.cpp:7
#2  0x00005555555551f7 in main () at test.cpp:5

Minor point: you don't need those parentheses around `*this`. In all the places where it's used, `*` binds more tightly than the rest of the expression, so the parentheses are redundant. — Pete Becker, Jun 13 '21 at 15:47
@PeteBecker i know that, it just helps me read the code a bit easier. thanks though — user898238409, Jun 13 '21 at 15:54
Self-assignment will cause some problems The assignment operator should guard against that, preferably by doing the steps in such a way that the heavy lifting happens first, and then changing the state of the object happens second, which will also help if an exception is thrown during the heavy lifting. — Eljay, Jun 13 '21 at 16:04
var::var(const char* v) { (*this) = v; } this makes my head spin. I am not sure what the compiler will do with it — Alessandro Teruzzi, Jun 13 '21 at 17:11

score 1 · Answer 1 · answered Jun 13 '21 at 15:56

Rule of 3\5 not followed so returning var from operator= in constructor results in shallow copy of var, The temporal object then gets deleted and it frees memory.

You have to

a) have implement copy constructor;

b) Fix that destructor, because it leaks memory if string is empty (but there is still memory allocated);

c) consider avoiding use of strdup and free and use new\delete and memcpy.

score 0 · Accepted Answer · answered Jun 13 '21 at 17:06

Your problem is that you are missing proper copy/move constructors and you are returning *this by value.

Your compiler makes a copy constructor for you which makes shallow copies and messes up your frees (you end up killing things that have already been killed).

Change the return type of your assignment operator to var& to return a reference and avoid a copy. Although that will fix your code, you are strongly advised to write your own copy/move constructors/assignment operators.

I would also advise you leave C behind and embrace the new/delete operators.

Finally, your use of the assignment operator here is misguided. First, the pointer reference type is superfluous. Second, you could move the logic of that operator to the constructor and you could still define v1 like you did in main without overloading =. You should instead overload it so that it accepts a const reference to type var to make a copy (ideally another overload to rvalue reference for move semantics).

thanks, that's a explanation. The part of new/delete, i know but as i said it's for learning purposes, i am trying to learn a bit of memory management and i want to sometime learn C, so i think malloc/free is better than new/delete(for later going deeper to C) — user898238409, Jun 13 '21 at 17:30
@user898238409 note that `strdup` isn't a part of C until a future (supposedly) standard, it's a POSIX function otherwise - and some platform lack it or have `_strdup` — Swift - Friday Pie, Jun 14 '21 at 08:01

"this" gets resetted C++

2 Answers2