Getting memory error "free(): invalid next size" for a particular size.

[kapil2905]

Hi ppl :),

I am working on implementing string using char.

I am facing a weird crash where if I populate my string more than a particular threshold, the program crashes with the following error.

I researched about this array and found that this error occurs due to invalid memory access or freeing memory twice. But I don't see any such issue in the code.

Error :
*** Error in `./testProg': free(): invalid next size (fast): 0x0000000002165c40


complete output :

s6 : hi there i am ki.sea grundy
*** Error in `./testProg': free(): invalid next size (fast): 0x0000000000ff7c40 ***


Here is the code :

str.h

#ifndef _STRING_H_
#define _STRING_H_

#include <iostream>
#include <memory>

class String final
{   
    public:
        String();
        String(const char *p);
        String(char *p);
        String(const String& s);
        unsigned long length() const;
        const char* to_cstr() const;
        String& operator =(const String& rhs);

    private:
        std::unique_ptr<char> ptr;

	//friends
        friend std::ostream& operator <<(std::ostream &out, const String& s);
        friend String operator +(const String& lhs, const char* p);
};

#endif 

str.cpp

#include "str.h"
#include "string.h"

String::String() {}

String::String(const char *p)
{
    ptr = std::make_unique<char>(strlen(p) + 1);
    strcpy(ptr.get(), p);
    ptr.get()[strlen(p)] = '\0';
}

String::String(char *p)
{
    ptr = std::make_unique<char>(strlen(p) + 1);
    strcpy(ptr.get(), p);
    ptr.get()[strlen(p)] = '\0';
}

String::String(const String& s) 
{
    ptr = std::make_unique<char>(s.length() + 1);
    strcpy(ptr.get(), s.to_cstr());
    ptr.get()[s.length()] = '\0';
}

const char* String::to_cstr() const
{
    return ptr.get();
}

unsigned long String::length() const
{
    return strlen(ptr.get());
}


/****************************  operator overloads   ***********************/
std::ostream& operator <<(std::ostream& out, const String& rhs)
{
    for (register int i = 0; i < rhs.length(); ++i)
        out << rhs.ptr.get()[i];

    return out;
}


String& String::operator =(const String& rhs)
{
    if (this == &rhs)   return *this;

    ptr.reset(nullptr);
    ptr = std::make_unique<char>(rhs.length() + 1);
    strcpy(ptr.get(), rhs.to_cstr());
    ptr.get()[rhs.length()] = '\0';

    return *this;
}

String operator +(const String& lhs, const char* p)
{
    char tmpStr[lhs.length() + strlen(p) + 1];
    strcpy(tmpStr, lhs.to_cstr());
    strcpy(tmpStr + lhs.length(), p);
    tmpStr[lhs.length() + strlen(p)] = '\0';
    return String{tmpStr};
}

main.cpp

#include "str.h"
#include <iostream>


int main()
{
    String s5 = "hi there i am ki.";
    
    String s6(s5 + "sea grundy");
    
    //works fine if I remove 1 or more characters from "sea grundy" !!
    //String s6(s5 + "sea grun");    //works fine. removed 'dy' from the end.

    std::cout << "\ns6 : " << s6 << std::endl;


    return 0;
}

I feel I am missing some very important thing/concept here.
Thanks alot for any kind of help :)

[mbozzi]

I feel I am missing some very important thing/concept here.

std::make_unique<char>(x) returns a unique_ptr referring to a single char, initialized to x. It will be created with new, not new[], and will be deleted with delete, not delete[]. The fix is to ask make_unique for an array of unknown bound, as in make_unique<char[]>(size).

Consider using std::vector<char> instead: this will save you from having to implement copy construction and copy assignment yourself.

Also:
char tmpStr[lhs.length() + strlen(p) + 1];
Will not compile. The size of an array must be a integral constant expression - i.e., an integer known at compile time.

[kapil2905]

Thanks @mbozzi for analysing this :)

But after applying your suggestions, I am getting a different error :(

I replaced
ptr = std::make_unique<char>(strlen(p) + 1);
with
ptr = std::make_unique<char[]>(strlen(p) + 1);

and getting error :

error: no match for ‘operator=’ (operand types are ‘std::unique_ptr<char>’ and ‘std::_MakeUniq<char []>::__array {aka std::unique_ptr<char [], std::default_delete<char []> >}’)
ptr = std::make_unique<char[]>(strlen(p) + 1);

and a big message pointing to library code.

I did check on internet and found that this syntax is valid from c++14, but not able to narrow down why I am getting this error :(

compile command I am using is :
g++ -C str.cpp -shared -fpic -o libstr.so -std=c++14
g++ version : version 6.3.0 20170519

Kindly lemme know if I am missing on something else.. :(

[JLBorges]

> I replaced ptr = std::make_unique<char>(strlen(p) + 1);
> with ptr = std::make_unique<char[]>(strlen(p) + 1);

Also replace std::unique_ptr<char> ptr; with std::unique_ptr< char[] > ptr; in the member declaration.

#include <iostream>
#include <memory>
#include <cstring>
#include <algorithm>

class String final
{
    public:
        String() { ptr[sz] = 0 ; }

        // note: member sz must be declared before ptr
        String( const char* p ) : sz( std::strlen(p) ) { std::copy( p, p+sz+1, ptr.get() ) ; }
        String( const String& that ) : sz(that.sz) { std::copy( that.to_cstr(), that.to_cstr()+sz+1, ptr.get() ) ; }
        String( String&& that ) noexcept { swap(that) ; } // move constructor

        // String(char *p); // not required

        // unifying assignment operator:
        // https://en.wikibooks.org/wiki/More_C++_Idioms/Copy-and-swap
        String& operator = ( String rhs ) noexcept { return swap(rhs) ; }

        ~String() = default ;

        String& operator += ( const String& rhs )
        {
            auto tmp = std::make_unique< char[] >( sz + rhs.sz + 1 ) ;
            std::copy( begin(), end(), tmp.get() ) ;
            std::copy( rhs.to_cstr(), rhs.to_cstr()+rhs.sz+1, tmp.get()+sz ) ;
            sz += rhs.sz ;
            using std::swap ; swap( ptr, tmp ) ;
            return *this ;
        }

        unsigned long length() const noexcept { return sz ; }
        const char* to_cstr() const noexcept { return ptr.get() ; }

        // right now: just to support range-based loops
        char* begin() noexcept { return ptr.get() ; }
        char* end() noexcept { return begin() + length() ; }
        const char* begin() const noexcept { return to_cstr() ; }
        const char* end() const noexcept { return begin() + length() ; }

        String& swap( String& that ) noexcept
        {
            using std::swap ;
            swap( sz, that.sz ) ;
            swap( ptr, that.ptr ) ;
            return *this ;
        }

    private:
        // note: member sz must be declared before ptr
        std::size_t sz = 0 ;
        std::unique_ptr< char[] > ptr = std::make_unique< char[] >(sz+1) ;

	friend std::ostream& operator <<( std::ostream &out, const String& str )
    { for( char c : str ) out << c ; return out ; }

    // canonical: see 'Binary arithmetic operators' in
    // http://en.cppreference.com/w/cpp/language/operators
    friend String operator + ( String lhs, const String& rhs ) { return lhs += rhs ; }
    friend String operator + ( String lhs, const char* p ) { return lhs += p ; }
    friend String operator + ( const char* p, const String& rhs )
    { return String(p) += rhs ; } // not the most efficient way of implementing this
};

void swap( String& a, String& b ) { a.swap(b) ; }

int main()
{
    const String s5 = "hi there i am ki.";
    const String s6 = "message: " + s5 + " sea grundy" ;
    String s7 ;
    s7 += "Just Testing!" ;

    std::cout << s6 << '\n' << s7 << '\n' ;

    for( char& c : s7 ) c = std::toupper(c) ;
    std::cout << s7 << '\n' ;
}

http://coliru.stacked-crooked.com/a/b177ad094fe18ece

[kapil2905]

Thanks alot @JLBorges to point that out and also to make me aware of many other concepts through the code above.