0

If you think I did not include anything potentially useful in this question feel free to comment and ask about it.

I have a simple Dna struct with 2 constructors where I allocate an array of int and a destructor where I delete this array (I do not refence this array anywhere else in the code). This is pretty simple, but it produces an invalid free.

#ifndef __DNA_H__
#define __DNA_H__

#include <random>
#include <cstddef>

#include "constants.hpp"

template <class URNG>
struct Dna
{
    Dna(const Dna& lhs, const Dna& rhs, URNG& urng) 
    {
        _data = new int[c_dnaLength];

        std::bernoulli_distribution startDistrib(0.5);
        std::bernoulli_distribution mutatDistrib(c_mutatP);
        std::bernoulli_distribution crossDistrib(c_crossP);

        const Dna * ptr = (startDistrib(urng)) ? &lhs : &rhs;

        for (std::size_t i = 0; i < c_dnaLength; i++)
        {
            if (crossDistrib(urng))
            {
                ptr = (ptr == &rhs) ? &lhs : &rhs;
            }

            _data[i] = mutatDistrib(urng) ? 1 ^ ptr->_data[i] : ptr->_data[i];
        }
    }

    Dna(URNG& urng)
    {
        _data = new int[c_dnaLength];

        std::uniform_int_distribution<int> dnaDistrib(0, 1);

        for (std::size_t i = 0; i < c_dnaLength; i++)
        {
            _data[i] = dnaDistrib(urng);
        }
    }

    ~Dna()
    {
        delete[] _data;
    }

    int* _data;
};

#endif

Here is a bit of the valgrind log :

==5084== Invalid free() / delete / delete[] / realloc()
==5084==    at 0x4C2C05C: operator delete[](void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5084==    by 0x40263F: Dna<std::mersenne_twister_engine<unsigned long, 32ul, 624ul, 397ul, 31ul, 2567483615ul, 11ul, 4294967295ul, 7ul, 2636928640ul, 15ul, 4022730752ul, 18ul, 1812433253ul> >::~Dna() (dna.hpp:47)

The destructor get call when I erase a member of a std::list of a class that contain a Dna struct member...so that's a bit complicated without showing large piece of code.

matovitch
  • 1,264
  • 11
  • 26
  • Can you please show how you use the class to invoke the error? – Some programmer dude Jan 25 '15 at 12:32
  • Show us a sample main() program that duplicates the error. Also, you are missing the user-defined assignment operator to handle copying of the data.. – PaulMcKenzie Jan 25 '15 at 12:33
  • :/ There is no copy constructor...I do not want an assignment operator or a copy constructor, I'm fine with the methods I got. – matovitch Jan 25 '15 at 12:34
  • 1
    Sorry, I was reading the constructor wrong. But that's probably the problem (together with the lack of copy-assignment operator). Read about [the rule of three](http://en.cppreference.com/w/cpp/language/rule_of_three) (or about [the rule of zero](http://flamingdangerzone.com/cxx11/2012/08/15/rule-of-zero.html)). – Some programmer dude Jan 25 '15 at 12:36
  • If that function is supposed to be a copy constructor, it doesn't make a *copy*. You're supposed to *duplicate* the object that is passed to you, unless you want a really scrambled program that will be a huge pain to debug. Edit, that is not a copy constructor, so you're missing that and an assignment operator. – PaulMcKenzie Jan 25 '15 at 12:37
  • That is not a copy constructor, that is a "breeding" constructor, I take two Dnas and "merge" them to produce a third piece of Dna. Again, I do not need a copy constructor...and if I need it, please explain me why. Thanks in advance ! – matovitch Jan 25 '15 at 12:40
  • 1
    @matovitch - I can easily make your program fall flat on its face with a simple 2 or 3 line program if you do *not* have the copy constructor and assignment operator. That's the reason why you need these functions. – PaulMcKenzie Jan 25 '15 at 12:42
  • Follow the rule of three/five, or just use a `std::vector` and follow the rule of zero. (If `c_dnaLength` is a compile-time constant, then just use an plain array.) Templating the class on the RNG's type also feels weird - are you sure you don't want to template the individual member functions instead? – T.C. Jan 25 '15 at 12:42
  • @PaulMcKenzie Can you show me the kind of program you are thinking of ? (I will read on the rule of three, thanks) – matovitch Jan 25 '15 at 12:44
  • @matovich any program that does copy assignment or copy construction will fail. – Öö Tiib Jan 25 '15 at 12:45
  • 1
    @matovitch - `{Dna x(whatever); Dna y(whatever); x = y;}` Boom. – PaulMcKenzie Jan 25 '15 at 12:45
  • @PaulMcKenzie Arf...Indeed, I do not do this in my code but the std container must do this kind of things. Thanks ! :) – matovitch Jan 25 '15 at 12:47
  • @matovitch - Assume that any code you write will make copies, even if you explicitly didn't write code to do so. The only way to ensure you're not making copies is to turn off copying at the compiling stage, and have the compiler emit an error if it detects that copying is done. Just ditch the `int*` in your class and use `std::vector`, and your issues will go away. – PaulMcKenzie Jan 25 '15 at 12:49
  • @PaulMcKenzie Yes, I opted for this last solution. Thanks ! – matovitch Jan 25 '15 at 12:54

0 Answers0