Do not delete a raw pointer that is not an owner (i.11)

Question

I was trying to experiment the rules of Five / Three / Zero. When Compiling my program I got a warning C26401saying that I have to not delete pointer that I don't own and this in the line delete[] pChar_; in the destructor function.

I agree that I have issue there specially when calling move ctor, when I apply std::swap when calling to the source object.

So how could I correct this issue ?

Edit: As suggested in comment, I may use std::string (Good) but I wanna know how to fix the issue without modifying the types let's say to learn that :)

using namespace std;

struct A
{

  A();
  A(int Int, const char* pChar);

  A(const A& rhs);
  A& operator=(A rhs);

  A(A&& rhs);

  ~A();

  void swap(A& rhs);
  int Int_ = 0;
  char *pChar_ = nullptr;
};

A::~A()
{
    std::cout << "----DTOR----" << std::endl;

    if (pChar_ != nullptr)
    {
        delete[] pChar_;
        pChar_ = nullptr;
    }
}

A::A(int Int, const char* pChar) :
     Int_(Int), pChar_(new char[strlen(pChar) + 1])
{
    strncpy_s(pChar_, strlen(pChar) + 1, pChar, _TRUNCATE);
}

A::A(const A& rhs) : A(rhs.Int_, rhs.pChar_) {}

A& A::operator=(A rhs)
{   
    swap(rhs);
    return *this;
}

A::A(A&& rhs)
{
    swap(rhs);
}

void A::swap(A& rhs)    
{
    std::swap(this->Int_, rhs.Int_);
    std::swap(this->pChar_, rhs.pChar_);
}

int main()
{
    A v1{ 1, "Hello" }; 
    {
        A v3{ std::move(v1) };
    }
}

Replace your `char*` with `std::string` and you can go back to Rule of Zero. — Cory Kramer, Nov 02 '18 at 18:16
@CoryKramer : yes this what I think to use, but I would like to know if there is way to fix that without using std::string, just to learn how fix such situations. — Blood-HaZaRd, Nov 02 '18 at 18:17
I'm not seeing a problem with the code. FWIW the `if (pChar_ != nullptr)` check is unneeded as `delete` on a `nullptr` is a non-op — NathanOliver, Nov 02 '18 at 18:19
@chris : nice I read that in the guideline but the example showed is sth like owner for a function returning a pointer but this is not my case, could you exlplain how could I use this Owner !?! — Blood-HaZaRd, Nov 02 '18 at 18:21
You would at least need to mark the pointer doing the owning (the one you delete). I don't know about parameters or anything. (edit: Never mind, it wouldn't apply to parameters if you're not seizing ownership anyway.) — chris, Nov 02 '18 at 18:26
unrelated: You've a perfect example here of [the toxicity of `using namespace std;`](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) by using your own `swap` and `std::swap`. — user4581301, Nov 02 '18 at 19:01

score 1 · Answer 1 · answered Nov 02 '18 at 19:06

I don't have copy of VS handy that can reproduce your warning but it seems to me that the warning is from the CPP Code Guidelines, specifically I.11: Never transfer ownership by a raw pointer (T*) or reference (T&):

Enforcement

(Simple) Warn on delete of a raw pointer that is not an owner<T>. Suggest use of standard-library resource handle or use of owner<T>.

So the solution is to use gsl::owner<char *> pChar_ = nullptr;. Please note that gsl::owner is nothing more than an annotation to help readers of code (humans or tools) and won't magically make your code safe.

As far as I can see your code looks ok.

One issue I have is the ctor:

A(int Int, const char* pChar) :
    Int_(Int), pChar_(new char[strlen(pChar) + 1])
{
    strncpy_s(pChar_, strlen(pChar) + 1, pChar, _TRUNCATE);
}

This is unsafe code disguised as safe. strncpy_s gives you a sense of safety. But in reality it does absolutely nothing (for safety that is) in your code. Because if there is a problem with pChar (e.g. it doesn't point to a null terminated string) then strlen will fail first. So either say what every std function says: pChar shall be a valid pointer to a null-termiated string and count as UB otherwise, or guard at strlen. Once you guard at strlen then strcpy is enough.

Also you don't check that Int is equal to strlen. They should be.

Do not delete a raw pointer that is not an owner (i.11)

1 Answers1