Ahmed Zafar
Ahmed Zafar

Reputation: 665

Resizing dynamic string array

I am trying to resize a dynamically allocated string array; here's the code!

void resize_array() {
    size_t newSize = hash_array_length + 100;
    string* newArr = new string[newSize];

    fill_n(hash_array,newSize,"0"); //fills arrays with zeros

    memcpy( newArr, hash_array, hash_array_length * sizeof(string) );

    hash_array_length = newSize;
    delete [] hash_array;
    hash_array = newArr;
}

unfortunately it isn't working and gives a segmentation fault. any idea why? this is basically a linear probing hash table where the element gets inserted wherever there is a 0 hence I use fill_n to fill the newly created array with 0's. any help please?

Upvotes: 1

Views: 2691

Answers (3)

StereoMatching
StereoMatching

Reputation: 5019

memcpy( newArr, hash_array, hash_array_length * sizeof(string) );

This line is extremely dangerous, std::string is not a plain old data type, you can't make sure that memcpy could initialize it correctly, it may cause undefined behavior, one of the most nasty behavior of c++(or programming).

Besides, there are a better and safer(in most of the times) solution to create a dynamic string array in c++, just use vector

//create a dynamic string array with newSize and initialize them with "0"
//in your case, I don't think you need to initialize it with "0"
std::vector<std::string> newArr(newSize, "0"); 

if the hash_array has the same type as newArr(std::vector) The way of copy it is very easy.

c++98

std::copy(hash_array.begin(), hash_array.end(), newArr.begin());

c++11

std::copy(std::begin(hash_array), std::end(hash_array), std::begin(newArr));

Better treat c++ as a new language, it has too many things are different from c. Besides, there are a lot of decent free IDE, like code::blocks and QtCreator devc++ is a almost dead project.

If you are new to c++, c++ primer 5 is a good book to start.

Upvotes: 4

milleniumbug
milleniumbug

Reputation: 15814

I suspect the culprit is memcpy call. string is complicated type which manages the char array by pointers (just as you are doing right now). Normally copying string is done using assignment operator, which for string also copies its own array. But memcpy simply copies byte-per-byte the pointer, and delete[] also deletes the array managed by string. Now the other string uses deleted string array, which is BAAAD.

You can use std::copy instead of memcpy, or even better yet, use std::vector, which is remedy to most of your dynamic memory handling problems ever.

Upvotes: 0

Jonathan Potter
Jonathan Potter

Reputation: 37162

If string is actually an std::string (and probably even if it isn't) then this will crash. You are creating a new array of strings, copying the old string classes over the top, and then freeing the old strings. But if the string class contains internal pointers to allocated memory this will result in a double free because all you are doing is copying the internal pointers - not making new memory allocations.

Think about it like this; imagine you had the following class:

class foo
{
    char* bar;

    foo() { bar = malloc(100); }
    ~foo() { free(bar);
};

foo* ptr1 = new foo;
foo* ptr2 = new foo;
memcpy(ptr2, ptr1, sizeof(foo*));
delete ptr1;

At this point, ptr2->bar points to the same memory that ptr1->bar did, but ptr1 and the memory it held has been freed

The best solution would be to use a std::vector because this handles the resizing automatically and you don't need to worry about copying arrays at all. But if you want to persist with your current approach, you need to change the memcpy call to the following:

for (int i = 0; i < hash_array_length; ++i)
{
    newArr[i] = hash_array[i];
}

Rather than just copying the memory this will call the class's copy constructor and make a proper copy of its contents.

Upvotes: 1

Related Questions