To reiterate: the buffer-overflow problem is the main reason why you should avoid C-style arrays (whether on the stack or the heap) and use proper container classes that manage memory for you.
Printable View
To reiterate: the buffer-overflow problem is the main reason why you should avoid C-style arrays (whether on the stack or the heap) and use proper container classes that manage memory for you.
The key point about my method is that the size of the array is decided at run time, not at compile time. You could of course do some calculations to determine nSize and create whatever size you want. When copying to this dynamic array you should use safe, truncating copy functions like _snprintf or strncpy, not sprintf or strcpy (or memcpy for other types).
You method does not work. It is very bad programming. You are creating a single character and trying to use it as an array. You might as well write
char* s = 0x01234567;
cin>>s;
Both will work and in both cases you are writting memory you are not ment to modify.
And you are right, there are problems with this method and it can be difficult to use, which is why the vector template was created.
//Edit: I got the picture. I went back and successful did the program using dynamic arrays and it worked.Quote:
Originally posted by mwilliamson
The key point about my method is that the size of the array is decided at run time, not at compile time. You could of course do some calculations to determine nSize and create whatever size you want.
Yep, I know. Just learning.Quote:
Your method does not work. It is very bad programming. You are creating a single character and trying to use it as an array. You might as well write
char* s = 0x01234567;
cin>>s;
Both will work and in both cases you are writting memory you are not ment to modify.
Yeah, I am discovering that not everything I am learning is good use. Again the term "dynamic array" threw me off and let me to expect more. So, for now I am sticking to container class till I get a better hand at this.Quote:
And you are right, there are problems with this method and it can be difficult to use, which is why the vector template was created.
Thanks for helping.
The above is a sample working code of what I was trying to do.Code:#include <iostream>
#include <string>
#include <fstream>
#include <vector>
using namespace std;
typedef unsigned short int uShort;
int main()
{
vector<char> vCh;
char ch;
char *arrayCh;
cout<<"\nEnter the name of the file to read: \t";
while(cin.get(ch))
{
if(ch == '\n')
break;
vCh.push_back(ch);
}
const uShort size = vCh.size();
arrayCh = new char[size];
for(uShort i=0; i<(uShort)vCh.size(); i++)
arrayCh[i] = vCh[i];
cout<<"\n\n";
ifstream in(arrayCh);
string s;
while(getline(in, s))
cout<<s <<"\n";
delete[] arrayCh;
arrayCh =0;
return 0;
}
I know I could have used:
... but it does not work well with file name with space in it.Code:string inFile;
cin>>inFile;
ifstream in(inFile.c_str());
The first code on top still has 3 issues:
1. The first issue is with cin.get(), I have to use cin.ignore(integer, '\n') if the code is in the middle of other operations to clean up the prompt.
2. How do I determine the size of the array arrayCh points to? (programmatically)
3. How can I be sure that nothing is being overriden as the array grows? I mean, what is the real mechanism being all this?
Ok, I know for a fact that the code is bad. I have found memory bugs in it.
There are problems if the file name is 1, 2, 4, 5, and 6 characters long - not 3 and above.
Trying to understand why.
c-cstrings need to be NULL terminated :
Code:arrayCh = new char[size+1];
for(uShort i=0; i<(uShort)vCh.size(); i++)
arrayCh[i] = vCh[i];
arrayCh[size] = 0;
Code:#include <iostream>
#include <string>
#include <fstream>
using namespace std;
int main()
{
cout<<"\nEnter the name of the file to read: \t";
string inFile;
std::getline(cin,inFile);
ifstream in(inFile.c_str());
if (in) cout << in.rdbuf() << endl;
return 0;
}
Thanks for the help.
I know your code works, but I have a need to solve the problem using dynamic arrays. My implementation of dynamic array in the above code is problematic at best. I am looking for someone to tell me why the code is shaddy.
And yes I forgot about "arrayCh = new char[size+1];", but the memory issues are still not solved.
Is the code below overriding memory it is not supposed to?
Code:for(uShort i=0; i<(uShort)vCh.size(); i++)
arrayCh[i] = vCh[i];
Looking at this code:
const uShort size = vCh.size();
arrayCh = new char[size];
for(uShort i=0; i<(uShort)vCh.size(); i++)
arrayCh[i] = vCh[i];
For one, you should use your size variable in the loop. While vCh.size() is unlikely to change, if it did you would have a buffer overflow. Plus, there is no reason to calculate the size every iteration of the loop. Especially when you have it stored!
I don't see anything immediately wrong with the code, maybe you could tell me what you mean by memory errors.
I know about vCh.size() in the for loop. I did that to test something.Quote:
Originally posted by mwilliamson
Looking at this code:
const uShort size = vCh.size();
arrayCh = new char[size];
for(uShort i=0; i<(uShort)vCh.size(); i++)
arrayCh[i] = vCh[i];
For one, you should use your size variable in the loop. While vCh.size() is unlikely to change, if it did you would have a buffer overflow. Plus, there is no reason to calculate the size every iteration of the loop. Especially when you have it stored!
I don't see anything immediately wrong with the code, maybe you could tell me what you mean by memory errors.
If you compile and run the code below, which is essentially the same code as the original and enter a file name that is 1, 2, 4, 5, and 6 character long, you will get an error. I have inserted a series of cout in order to determine the root of the problem. So, take a look at the output and you might see the root cause of the problem.
Code:#include <iostream>
#include <string>
#include <fstream>
#include <vector>
using namespace std;
typedef unsigned short int uShort;
int main()
{
vector<char> vCh;
char ch;
char *arrayCh;
cout<<"\nEnter a choice: \t";
while(cin.get(ch))
{
if(ch == '\n')
break;
vCh.push_back(ch);
cout<<ch <<endl; //checking
}
const uShort size = vCh.size();
cout<<"size = " <<size <<endl; //checking
cout<<"size = " <<vCh.size() <<endl; //checking
arrayCh = new char[size + 1];
cout<<"arrayCh = " <<sizeof(arrayCh) <<"\tcontent: " <<arrayCh <<"\t\taddress: " <<&arrayCh <<endl;//checking
cout<<"arrayCh[] = " <<sizeof(arrayCh[size]) <<"\tcontent: " <<arrayCh[size] <<"\t\taddress: " <<&arrayCh[size] <<endl;
for(uShort i=0; i<(uShort)vCh.size()/*(size)checking*/; i++)
arrayCh[i] = vCh[i];
cout<<"arrayCh = " <<sizeof(arrayCh) <<"\tcontent: " <<arrayCh <<"\taddress: " <<&arrayCh <<endl;//checking
cout<<"arrayCh[] = " <<sizeof(arrayCh[size]) <<"\tcontent: " <<arrayCh[size] <<"\t\taddress: " <<&arrayCh[size] <<endl;
cout<<"\n\n";
ifstream in(arrayCh);
string s;
while(getline(in, s))
cout<<s <<"\n";
delete[] arrayCh;
arrayCh =0;
return 0;
}
Here is the output when the file name is 2 characters (vc)
As opposed to this when the file name is 3 characters long (cvc) or greater than 6 characters:Code:
E:\>dynamic
Enter a choice: vc
v
c
size = 2
size = 2
arrayCh = 4 content: ─¥C address: 1244660
arrayCh[] = 1 content: C address: C
arrayCh = 4 content: vcC address: 1244660
arrayCh[] = 1 content: C address: C
E:\>
Code:
E:\>dynamic
Enter a choice: cvc
c
v
c
size = 3
size = 3
arrayCh = 4 content: ─¥C address: 1244660
arrayCh[] = 1 content: address:
arrayCh = 4 content: cvc address: 1244660
arrayCh[] = 1 content: address:
-VC
1-2 vc
2-1 c
3-0
4-3 -vc
5-2 vc
6-1 c
7-0
8-0
9-0
10-0
11-0
E:\>
- you forgot to set the last character to NULL
as in my previous post :
Code:arrayCh = new char[size+1];
for(uShort i=0; i<(uShort)vCh.size(); i++)
arrayCh[i] = vCh[i];
arrayCh[size] = 0; // you forgot this line
- another way ... push NULL unto the vector array:
Code:while(cin.get(ch))
{
if(ch == '\n')
break;
vCh.push_back(ch);
}
vCh.push_back(0); // add this line
const uShort size = vCh.size();
arrayCh = new char[size]; // no need for + 1
strcpy(arrayCh,&vCh[0]); // note the syntax of using the vector
- even better, forget about the arrayCh completely :
Code:while(cin.get(ch))
{
if(ch == '\n')
break;
vCh.push_back(ch);
}
vCh.push_back(0); // add this line
cout<<"\n\n";
ifstream in(&vCh[0]); // use the vector (again note syntax)
code1: does not work still (corrected arrayCh[size + 1] = 0;)
code2: works well <= still worry about memory being overriden somewhere
code3: works well <= but I needed dynamic array implementation in the code.
Do you know any better way of getting inputs off the command prompt than cin.get() and getline()?
Thanks a bunch for the help. :)