c++ - Copy doesn't work in my string::copy implementation -

following on https://codereview.stackexchange.com/q/126242/23788.

i wrote string class , according feedback have changed stuff. there more should fixed?

+operator doesn't work , not know i've done wrong. have segfault when "str+str".

process finished exit code 139 

and str.h

class str {      friend std::istream &operator>>(std::istream &, str &);      friend void swap(str &s, str &t) {         std::swap(s.data, t.data);         std::swap(s.length, t.length);         std::swap(s.alloc, t.alloc);     }   public:     typedef char *iterator;     typedef size_t size_type;      str() : data(nullptr), length(0), capacity(0) { }      str(size_type length, char char_to_fill) : str() { create(length, char_to_fill); }      str(const char *s) : str() { create(s); }      template<class in>     str(in b, in e) : str() { create(b, e); }      ~str() {         if (data) alloc.deallocate(data, capacity);         data = nullptr;     }      str(const str &s) {         *this = s;     }      // move constructor?     str(str &&other)             : str() {// initialize via default constructor, c++11         swap(*this, other);     }      str &operator+=(const str &s) {         size_type new_length = length + s.length - 1; //remove 1 because of 2 nulls         if (new_length > capacity) {             reallocate(new_length);             strcpy(data + length - 1, s.data); //overwrite null s             length = new_length;         }         else {//if there enough space             strcpy(data + length - 1, s.data);         }         return *this;     }      str &operator=(str rhs) {         swap(*this, rhs);         return *this;     }      char &operator[](size_type i) { return data[i]; };      const char &operator[](size_type i) const { return data[i]; };      size_type size() { return length; }      const size_type size() const { return length; }      const char *c_str() const {         return data;     }      void copy(char *dest, size_type n) {         if (n > length)             throw std::out_of_range("out of range");         std::copy(data, data + n, dest);     }      char *begin() { return data; };      char *end() { return data + length; };      void push_back(char c) {         if (length == capacity) {             reallocate(capacity == 0 ? default_capacity : 2 * capacity);         }         data[length++] = c;     }  private:     char *data;     std::allocator<char> alloc;     size_type length;     size_type capacity;     static const size_type default_capacity = 20;      void create(size_type n, char character_to_fill) {         capacity = length = n + 1;         data = alloc.allocate(capacity);         std::uninitialized_fill(data, data + length - 1, character_to_fill);         //alloc.construct(data + length - 1, '\0'); //is needed constructed?         data[length - 1] = '\0';     }      void create(const char *s) {         capacity = length = strlen(s) + 1;         data = alloc.allocate(capacity);         strcpy(data, s);         //alloc.construct(data + length - 1, '\0');         data[length - 1] = '\0';     }      template<class in>     void create(in b, in e) {         capacity = e - b + 1;         data = alloc.allocate(capacity);         while (b != e) {             data[length++] = *(b++);         }         //alloc.construct(data + length -1, '\0');         data[length++] = '\0';     }      void reallocate(size_t new_capacity) {         char *new_data = alloc.allocate(new_capacity);         std::copy(data, data + length, new_data);         alloc.deallocate(data, length);         data = new_data;         capacity = new_capacity;     } };  std::istream &operator>>(std::istream &is, str &s) {     std::vector<char> buf;     char actual_character;     while (is.get(actual_character) && isspace(actual_character)) { ;     }     if (is) { //is correct check "is" ?         buf.push_back(actual_character);         while (is.get(actual_character) && !isspace(actual_character));         if (is)             is.unget();     }     s.create(buf.begin(), buf.end());     return is; }  std::ostream &operator<<(std::ostream &os, const str &s) {     os << s.c_str();     return os; }  str operator+(str lhs, const str &rhs) {     lhs += rhs;     return lhs; } 

and example main.cpp

#include <iostream> #include <vector> #include "str.h"  using std::cout; using std::endl;  int main() {     str s("siema");     cout<<s.c_str()<<endl;      s = "hello";     cout<<s<<endl;      s.push_back('a');     cout<<s<<endl;      str t = "world";     //cout<<s+t<<endl; //this doesnt work      s+=t;     cout<<s<<endl;      cout<<s[3]<<s[5]<<s[11]<<endl;      cout<<s.size()<<endl;     cout<<str(s.begin()+3, s.end()-2)<<endl;     for(str::iterator = s.begin(); i<s.end() ; i+=2){         cout<<i<<endl;     }      char copied[3];     t.copy(copied, 4);     cout<<copied<<endl;      return 0; } 

in code

char copied[3]; t.copy(copied, 4); cout<<copied<<endl; 

"copied" has length of 3 while trying copy 4 characters it. cause problem

check updated codes below. read comments "<--"

str.h

#include <iostream> #include <memory> #include <vector> class str {  friend std::istream &operator >> (std::istream &, str &);   void swap(str &s, str &t) {     std::swap(s.data, t.data);     std::swap(s.length, t.length);     std::swap(s.alloc, t.alloc); }   public: typedef char *iterator; typedef size_t size_type;  str() : data(nullptr), length(0), capacity(0) { }  str(size_type length, char char_to_fill) : str() { create(length, char_to_fill); }  str(const char *s) : str() { create(s); }  template<class in> str(in b, in e) : str() { create(b, e); }  ~str() {     if (data) alloc.deallocate(data, capacity);     data = nullptr; }  str(const str &s) {     *this = s; }  // move constructor? str(str &&other)     : str() {// initialize via default constructor, c++11     swap(*this, other); }  str &operator+=(const str &s) {     size_type new_length = length + s.length - 1; //remove 1 because of 2 nulls     if (new_length > capacity) {         reallocate(new_length);         strcpy(data + length - 1, s.data); //overwrite null s         //length = new_length;  //<-- need update length anyay. move before return     }     else {//if there enough space         strcpy(data + length - 1, s.data);     }     length = new_length; //<-- update length     return *this; }  str &operator=(str rhs) {     swap(*this, rhs);     return *this; }  char &operator[](size_type i) { return data[i]; };  const char &operator[](size_type i) const { return data[i]; };  size_type size() { return length; }  const size_type size() const { return length; }  const char *c_str() const {     return data; }  void copy(char *dest, size_type n) {     if (n > length)         throw std::out_of_range("out of range");     std::copy(data, data + n, dest);  // <--forgot '\0'?     dest[n] = '\0';                   // <-- add '\0' }  char *begin() { return data; };  char *end() { return data + length; };  void push_back(char c) {     if (length == capacity) {         reallocate(capacity == 0 ? default_capacity : 2 * capacity);     }     data[length++ - 1] = c;  //<-- length - 1 last position, because length here includes '\0'     data[length - 1] = 0;    //<-- don't forget add '\0'. it's better if fill unused spaces '\0' after allocate them. }  private: char *data; std::allocator<char> alloc; size_type length; size_type capacity; static const size_type default_capacity = 20;  void create(size_type n, char character_to_fill) {     capacity = length = n + 1;     data = alloc.allocate(capacity);     std::uninitialized_fill(data, data + length - 1, character_to_fill);     //alloc.construct(data + length - 1, '\0'); //is needed constructed?     data[length - 1] = '\0'; }  void create(const char *s) {     capacity = length = strlen(s) + 1;     data = alloc.allocate(capacity);     strcpy(data, s);     //alloc.construct(data + length - 1, '\0');     data[length - 1] = '\0'; }  template<class in> void create(in b, in e) {     capacity = e - b + 1;     data = alloc.allocate(capacity);     while (b != e) {         data[length++] = *(b++);     }     //alloc.construct(data + length -1, '\0');     data[length++] = '\0'; }  void reallocate(size_t new_capacity) {     char *new_data = alloc.allocate(new_capacity);     std::copy(data, data + length, new_data);     alloc.deallocate(data, length);     data = new_data;     capacity = new_capacity; } };  std::istream &operator >> (std::istream &is, str &s) { std::vector<char> buf; char actual_character; while (is.get(actual_character) && isspace(actual_character)) {     ; } if (is) { //is correct check "is" ?     buf.push_back(actual_character);     while (is.get(actual_character) && !isspace(actual_character));     if (is)         is.unget(); } s.create(buf.begin(), buf.end()); return is; }  std::ostream &operator<<(std::ostream &os, const str &s) { os << s.c_str(); return os; }  str operator+(str lhs, const str &rhs) { lhs += rhs; return lhs; } 

and main:

int main() { str s("siema"); cout << s.c_str() << endl;  s = "hello"; cout << s << endl;  s.push_back('a'); cout << s << endl;  str t = "world"; //cout<<s+t<<endl; //this doesnt work  s += t; cout << s << endl;  cout << s[3] << s[5] << s[11] << endl;  cout << s.size() << endl; cout << str(s.begin() + 3, s.end() - 2) << endl; (str::iterator = s.begin(); i<s.end(); += 2) {     cout << << endl; }  char copied[5];  //<-- 3, not enough space t.copy(copied, 4); cout << copied << endl;  return 0; }