Vector of C style strings

[twocups]

I really need some help. this is a simple program, its supposed to create a vector of c style strings. something is wrong though, if you look at the output, it is filling my vector of c-style strings with the last cstyle string that i generated.
PS. i am using c++11 so if you have older versions you wont b able to run this becouse of my random number generator.

#include<random>
#include<vector>
#include<functional>
#include<cstdlib>
#include<iostream>
#include<algorithm>
#include <cstring>
#include <string>
// generates random c style strings, and stores them in a vector, at least it is supposed to do that
using namespace std;
int main( )
{
    auto gen = bind(normal_distribution<double>(13,4),default_random_engine()); // Normal distribution random number generator
	int vectorsize =10;
    char strcnv[9]={'\0'};
    char* cstrcnv;
	vector <char*> cst (vectorsize);   //string vector

	for (int j =0; j<(vectorsize); ++j) //populate the string vector
	{
	    for (int k = 0; k<8; ++k)
	    {
	    	int charname = gen();
	    	strcnv[k] = (char)(((int) 'A') + charname);
	    	if (k%8==0)
	    	{
        cstrcnv = strcnv;
        cst[j] =  cstrcnv ;
	    	}
	    }
        cout<< " inside the loop "<<j<<"   "<<cst[j]<<endl;
	}
    for (int i = 0; i < vectorsize; i++)
    {
        cout<<" outside the loop "<<i<<"   "<<cst[i]<<"    "<<endl;
    }
return 0;
}

my output outside of the loop is just the last generated random c-style string that has populated my vector.

[cire]

char strcnv[9]={'\0'}

This is the only space allocated for any c-style strings.

What you are doing is continually reusing that space to generate different strings (overwriting previously generated strings,) while you insert the same address into the vector multiple times.

Since every pointer in the vector is pointing to the same memory, the output is not surprising.

[twocups]

I realized that, but i cant think of how to this task correctly. i tried diffirent things, im a novice. any suggestions would be extremely appreciated.

[twocups]

i forgot to mention, even though it is a vector of only 10 elements here, I made it small for the sake of debugging. it is potentially a very large vector.

[o1vk4iN6]

Allocate memory on the heap isntead of the stack ?

cst[ j ] = new char[ 9 ];

[twocups]

i had already tried that, i tried it again. it didn't make a difference.

[o1vk4iN6]

You probably used it wrong then, post code.

[twocups]

#include<random>
#include<vector>
#include<functional>
#include<cstdlib>
#include<iostream>
#include<algorithm>
#include <cstring>
#include <string>
// generates random c style strings, and stores them in a vector, at least it is supposed to do that
using namespace std;
int main( )
{
    auto gen = bind(normal_distribution<double>(13,4),default_random_engine()); // Normal distribution random number generator
	int vectorsize =10;
    char strcnv[9]={'\0'};
    char* cstrcnv;
	vector <char*> cst (vectorsize);   //string vector

	for (int j =0; j<(vectorsize); ++j) //populate the string vector
	{
	    for (int k = 0; k<8; ++k)
	    {
	    	int charname = gen();
	    	strcnv[k] = (char)(((int) 'A') + charname);
	    	if (k%8==0)
	    	{
        cst[ j ] = new char[ 9 ];
        cstrcnv = strcnv;
        cst[j] =  cstrcnv ;
	    	}
	    }
        cout<< " inside the loop "<<j<<"   "<<cst[j]<<endl;
	}
    for (int i = 0; i < vectorsize; i++)
    {
        cout<<" outside the loop "<<i<<"   "<<cst[i]<<"    "<<endl;
    }
return 0;
}

[o1vk4iN6]

cst[ j ] = new char[ 9 ];
cst[ j ] =  cstrcnv ;

What do you expect cst[ j ] to be after these 2 lines are run ? (you are not creating a copy) You are also creating a memory leak by overwriting the pointer before freeing the memory.

http://www.cplusplus.com/doc/tutorial/dynamic/

http://www.cplusplus.com/reference/clibrary/cstring/strncpy/

Although rather then having "strcnv" you can just directly place it onto the allocated memory rather then copying it after.

You can also just do:

std::vector< std::string > cst;

Avoiding for you to handle allocation and deallocation, then a copy will be made when you do the assignment.

[twocups]

what about this? it seems to work, but im not comfortable, there is probably a memory leak.

#include<random>
#include<vector>
#include<functional>
#include<cstdlib>
#include<iostream>
#include<algorithm>
#include <cstring>
#include <string>
// generates random c style strings, and stores them in a vector, at least it is supposed to do that
using namespace std;
int main( )
{
    auto gen = bind(normal_distribution<double>(13,4),default_random_engine()); // Normal distribution random number generator
	int vectorsize =10;
   // char strcnv[9]={'\0'};
    char* cstrcnv;
	vector <char*> cst (vectorsize);   //string vector

	for (int j =0; j<(vectorsize); ++j) //populate the string vector
	{
       cstrcnv = new char[1000];
	    for (int k = 0; k<8; ++k)
	    {
	    	int charname = gen();
	    	cstrcnv[k] = (char)(((int) 'A') + charname);
	    	strcat(cstrcnv, "\0");
	    }
        cst[ j ] = cstrcnv;

        cout<< " inside the loop "<<j<<"   "<<cst[j]<<endl;
	}
    for (int i = 0; i < vectorsize; i++)
    {
        cout<<" outside the loop "<<i<<"   "<<cst[i]<<"    "<<endl;
    }
return 0;
}

[cire]

There is a memory leak as you never deallocate the memory you allocated. Also allocating a 1000 char array for each 9 character string seems a little wasteful. If you can't go with std::string for whatever reason, you should definitely use std::unique_ptr.

#include<random>
#include<vector>
#include<functional>
#include<iostream>
#include <memory>

int main( )
{
    auto gen = std::bind(std::normal_distribution<double>(13,4),std::default_random_engine());

    typedef std::unique_ptr<char []> ptr_t ;
    std::vector<ptr_t> strings ; //string vector

    const unsigned strings_to_gen = 10 ;
    const unsigned string_size = 9 ;

    for ( unsigned i=0;  i < strings_to_gen ;  ++i )
    {
        strings.push_back( ptr_t(new char[string_size]) ) ;
        char* working = strings.back().get() ;

        for ( unsigned j=0; j < string_size-1 ; ++j )
            working[j] = static_cast<char>(gen()) + 'A' ;

        working[string_size-1] = '\0' ;

        std::cout << "inside the loop: " << i << ' ' << strings[i].get() << std::endl ;
    }
 

    for ( unsigned i=0; i<strings.size(); ++i )
        std::cout << "outside the loop " << i << " " << strings[i].get() << std::endl ;
    return 0;
}

[Cubbi]

typedef std::unique_ptr<char> ptr_t 
...
ptr_t(new char[string_size])

that's UB. make it
typedef std::unique_ptr<char[]> ptr_t

[cire]

Oops. Corrected.