Search code examples
c++pointersvector

Can't delete elements from homemade vector


I have task to do a simplified version of std::vector. My program is being tested on the server and it can't pass "test resize(with valgrind)". Here is implementation of my vector:

my_vector_impl.h

#include "my_vector.h"
#include <type_traits>
#include <algorithm>
#include <stdexcept>
#include <cmath>

namespace containers {

    template <typename T>
    my_vector<T>::my_vector() {
        capacity_ = 1;
        size_ = 0;
        array_ = reinterpret_cast<T*>(::operator new (sizeof(T)));
    }

    template <typename T>
    my_vector<T>::my_vector(std::size_t n) {
        if (!std::is_default_constructible<T>::value) {
            throw std::runtime_error("Cannot use non default constructible type!");
        }
        capacity_ = (size_t)pow(2, ceil(log2(n)));  // minimum power of two
        size_ = n;
        array_ = reinterpret_cast<T*>(::operator new (sizeof(T) * capacity_));

        for (size_t i = 0; i < size_; i++) {
            new (array_ + i) T();
        }
    }

    template <typename T>
    my_vector<T>::~my_vector() {
        for (size_t i = 0; i < size_; i++) {
            array_[size_ - i - 1].~T();
        }
        ::operator delete(array_);
    }

    template <typename T>
    void my_vector<T>::resize(std::size_t n) {
        if (!std::is_default_constructible<T>::value) {
            throw std::runtime_error("Cannot use non default constructible type!");
        }
        if (n <= size_) {
            for (size_t i = size_ - 1; i > n - 1; i--) {
                array_[i].~T();
            }
            size_ = n;
        }
        else if (n > size_ && n <= capacity_) {
            for (size_t i = size_; i < n; i++) {
                new (array_ + i) T();
            }
            size_ = n;
        }
        else if (n > size_ && n > capacity_) {
            reserve(n);
            for (size_t i = size_; i < n; i++) {
                new (array_ + i) T();
            }
            size_ = n;
        }
    }

    template <typename T>
    void my_vector<T>::reserve(std::size_t n) {
        if (n > capacity_) {
            size_t new_capacity = (size_t)pow(2, ceil(log2(n)));
            T* new_array = reinterpret_cast<T*>(::operator new(sizeof(T) * new_capacity));

            for (size_t i = 0; i < size_; i++) {
                new (new_array + i) T(array_[i]);
                array_[i].~T();
            }

            capacity_ = new_capacity;
            std::swap(array_, new_array);
            ::operator delete (new_array);
        }
    }
}

And some artifacts from server tester:

<system-err>==2162058== Memcheck, a memory error detector
==2162058== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2162058== Using Valgrind-3.18.1 and LibVEX rerun with -h for copyright info
==2162058== Command: lab-11_vector/lab-11_vector test_resize
==2162058== 
==2162058== 
==2162058== HEAP SUMMARY:
==2162058==     in use at exit: 96 bytes in 17 blocks
==2162058==   total heap usage: 119 allocs, 102 frees, 79,640 bytes allocated
==2162058== 
==2162058== 2 bytes in 2 blocks are definitely lost in loss record 1 of 2
==2162058==    at 0x484A2F3: operator new[](unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2162058==    by 0x1109E3: containers::my_vector<Product>::resize(unsigned long) (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058==    by 0x1127BB: void test_resize<Product>() (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058==    by 0x10A6C5: main (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058== 
==2162058== 94 bytes in 15 blocks are definitely lost in loss record 2 of 2
==2162058==    at 0x484A2F3: operator new[](unsigned long) (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2162058==    by 0x10E25A: containers::my_vector<Product>::reserve(unsigned long) (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058==    by 0x1109A1: containers::my_vector<Product>::resize(unsigned long) (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058==    by 0x1127BB: void test_resize<Product>() (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058==    by 0x10A6C5: main (in /var/lib/gitlab-runner/builds/4WbXJi1U/0/...)
==2162058== 
==2162058== LEAK SUMMARY:
==2162058==    definitely lost: 96 bytes in 17 blocks
==2162058==    indirectly lost: 0 bytes in 0 blocks
==2162058==      possibly lost: 0 bytes in 0 blocks
==2162058==    still reachable: 0 bytes in 0 blocks
==2162058==         suppressed: 0 bytes in 0 blocks
==2162058== 
==2162058== For lists of detected and suppressed errors, rerun with: -s
==2162058== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
</system-err>

I think my problem caused by "new_array" in "my_vector::reserve()". If I missed something (methods, files, etc.) please tell me.

I tried to change this code:

std::swap(array_, new_array);
::operator delete (new_array);

to this:

operator delete (array_);
array_ = new_array;

But nothing changed.


Solution

  • Consider what happens to your code if n equals zero (and size_ is greater than zero).

       if (n <= size_) {
            for (size_t i = size_ - 1; i > n - 1; i--) {
                array_[i].~T();
            }
            size_ = n;
        }
    

    In that case n - 1 is an integer overflow, since n is unsigned. Rewrite the loop to avoid these overflows, like this

       if (n <= size_) {
            for (size_t i = size_; i > n; i--) {
                array_[i - 1].~T();
            }
            size_ = n;
        }
    

    The moral is always be very careful doing subtraction on unsigned values.