Error when transfering the value of a pointer, using CLI

Question

I am trying to make a very dummy program, where I insert the values in prepare and search in the search. I am very new to C++, and I am just trying to learn some concepts of the language. When I try to vector = *v, I receive an error: Attempted to read or write protected memory. This is often an indication that other memory is corrupt. I am using vc++ to use in a c++/cli experiment.

What am I doing wrong?

#include "stdafx.h"
#include <algorithm>
#include <vector>
#include <ostream>
#include <iostream>

#pragma once

class BinarySearch
{
public:

    BinarySearch(void)
    {
    }

    virtual ~BinarySearch(void)
    {
    }

    int search(int key)
    {
         std::vector<int>::iterator low,up;
         low=std::lower_bound (vector.begin(), vector.end(), key); 

         return low - vector.begin();
    }

    std::vector<int> vector;

    void prepare(void)
    {
        std::vector<int>* v = 
            new std::vector<int>();

        int max = std::pow(33, 3);

        for(int i=0; i < max; i++) {
            v->push_back(i);
        }

        vector = *v;
    }
};

The prepare method is used before the the search.

The wrapper, which calls the code in c++:

namespace Native {
    public ref class Wrapper
    {
    public:

        Wrapper(void)
        {
        }

        BinarySearch* b;

        void Prepare(void)
        {
            b->prepare();       
        }

        int Search(int i)
        {
            return b->search(i);
        }
    };
}

Caller in c#:

class Program
{
    static void Main(string[] args)
    {
        var w = new Wrapper();
        w.Prepare();
        var position = w.Search(12);
        Console.WriteLine("Array.BinarySearch p: {0}", position);
        Console.ReadLine();
    }
}

Answer 1

You have to delete the vector you create with the new. The line vector = *v just copies the data in the other one. In other words, a memory leak! so call delete v afterwards.

Edit: Why not just not use a pointer if you're going to use it this way? Also, you could simply use the vector and clear it at the start of the function that changes its content anyways.

If you really need a pointer, declare the member variable a pointer first, make sure to initialize at NULL and when calling prepare, delete pointer then assign the new vector you created with the new. Note that calling delete on a NULL pointer results as noop (no operation). Though at this point... you might just want to call new once in the constructor and delete in destructor. You just need to call clear in prepare to clear the content.

This is what your code should look like:

std::vector<int> vector;

void prepare(void)
{
    std::vector<int>* v = 
    new std::vector<int>();

    int max = std::pow(33, 3);

    for(int i=0; i < max; i++) {
        v->push_back(i);

    vector = *v;
    delete v;
}

OR

std::vector<int> vector;

void prepare(void)
{
    vector.clear();

    int max = std::pow(33, 3);

    for(int i=0; i < max; i++) {
        vector.push_back(i);
    }
}

Edit: As Hans said, you didn't assign anything to your b pointer in the wrapper. Check his answer for more details.

Answer 2

You just forgot to assign the b pointer. So of course your code will crash when you dereference it in your Prepare() wrapper method. Simply create an instance of the BinarySearch class in your constructor.

You will also need to ensure that the native object gets destroyed again. Implement both the destructor and the finalizer so this is guaranteed to be done, even when the C# code doesn't dispose the object. Like this:

public ref class Wrapper 
{
private:
    BinarySearch* b;
public:
    Wrapper(void) : b(new BinarySearch) {}
    ~Wrapper() { delete b; b = nullptr; }
    !Wrapper() { this->~Wrapper(); }

    void Prepare(void) {
        if (b == nullptr) throw gcnew ObjectDisposedException("Wrapper");
        b->prepare();       
    }
    // etc...
};