2

Ok so here are the parts of my code that I'm having trouble with: 

char * historyArray;
historyArray = new char [20];

//get input
cin.getline(readBuffer, 512);       
cout << readBuffer <<endl;

//save to history
for(int i = 20; i > 0; i--){
    strcpy(historyArray[i], historyArray[i-1]); //ERROR HERE//  
}

strcpy(historyArray[0], readBuffer); //and here but it's the same error//

The error that i'm receiving is: 

"invalid conversion from 'char' to 'char*' 
           initializing argument 1 of 'char* strcpy(char*, const char*)'

The project is to create a psudo OS Shell that will catch and handle interrupts as well as run basic unix commands. The issue that I'm having is that I must store the past 20 commands into a character array that is dynamically allocated on the stack. (And also de-allocated)

When I just use a 2d character array the above code works fine:

char historyArray[20][];

but the problem is that it's not dynamic...

And yes I do know that strcpy is supposed to be used to copy strings.

Any help would be greatly appreciated!

Robᵩ
  • 163,533
  • 20
  • 239
  • 308
OmegaTwig
  • 243
  • 1
  • 4
  • 15

7 Answers7

7

historyArray points to (the first element of) an array of 20 chars. You can only store one string in that array.

In C, you could create a char** object and have it point to the first element of an array of char* objects, where each element points to a string. This is what the argv argument to main() does.

But since you're using C++, it makes a lot more sense to use a vector of strings and let the library do the memory management for you.

Keith Thompson
  • 254,901
  • 44
  • 429
  • 631
1

Two solutions. The first is if you for some reason really want arrays, the other is more recommended and more "C++"ish using std::strings.

char * historyArray[20]; // Create an array of char pointers

// ...

historyArray[i] = new char[SIZE]; // Do this for each element in historyArray

Then you can use strcpy on the elements in historyArray.

Second solution which I repeat is recommended (I've fixed a few other things):

string historyArray[20];

getline(cin, readBuffer); // Make readbuffer an std::string as well
cout << readBuffer << endl;

for(int i = 19; i > 0; i--){ // I think you meant 19 instead of 20
    historyArray[i] = historyArray[i-1];
}

historyArray[0] = readBuffer;
flight
  • 7,162
  • 4
  • 24
  • 31
1

Stop using C idioms in a C++ program:

std::deque<std::string> historyArray;

//get input
std::string readBuffer;
std::getline(std::cin, readBuffer);       
std::cout << readBuffer << std::endl;

//save to history
historyArray.push_front(readBuffer);
if(historyArray.size() > 20)
  historyArray.pop_back();

As a result, we have:

  • No buffer-overflow threat in readBuffer / getline()
  • No pointers, anywhere, to confuse us.
  • No arrays to overstep the ends of
  • Arbitrarily long input strings
  • Trivially-proven memory allocation semantics
Robᵩ
  • 163,533
  • 20
  • 239
  • 308
  • Thanks! My professor's wording of the project assignment led most of the class to believe that he wanted us to use native C strings. I talked to him and he said he didn't care. – OmegaTwig Sep 28 '11 at 15:56
  • You are welcome. Please do yourself a favor -- in future questions that derive from classroom assignments or other learning exercises, please add the "homework" tag. Doing so helps people like me not to blurt out an answer (as I did here.) In "homework"-tagged questions, people take greater care to explain their logic and reasoning, instead of just providing an answer. – Robᵩ Sep 28 '11 at 16:07
0

Error 1: You're indexing past your array bounds with i being set to 20.

Error 2: historyArray[i] is a char, not a char *. You need &historyArray[i].

Carey Gregory
  • 6,836
  • 2
  • 26
  • 47
  • 1
    That won't solve the problem. He needs to store multiple strings, but he only has one char array. Adding `&` operators to the arguments will attempt to copy overlapping strings, which has undefined behavior. – Keith Thompson Sep 27 '11 at 21:34
  • Yeah, you're right. I read the question too quickly. But he still needs to fix the errors I listed after he converts historyArray to an array of pointers or uses a vector. – Carey Gregory Sep 27 '11 at 21:36
  • Keith is correct. Good catch on out-of-bounds access though. Note that he does access `historyArray[0]` twice. – avakar Sep 27 '11 at 21:36
0
strcpy(&historyArray[i], &historyArray[i-1]);

Array notation gives references while strcopy wants pointers. Convert references to pointers with address-of (&) operator.

Robᵩ
  • 163,533
  • 20
  • 239
  • 308
Pubby
  • 51,882
  • 13
  • 139
  • 180
0

historyArray[i] is a char. It is a single character. You want to use a sting. Your fundemental problem is that historyArray is a char* which means that it points to a memory range containing characters. You want it to be a char** which is a pointer to a pointer to a string. Your initialization code would be 

char** historyArray;
historyArray = new char* [20];
for (int i = 0; i < 20; i++)
{
    historyArray[i] = new char [512];  //Big enough to have a 512 char buffer copied in
}
Konstantin Tarashchanskiy
  • 2,762
  • 2
  • 31
  • 46
0
char * historyArray;
historyArray = new char [20];

//get input
cin.getline(readBuffer, 512);       
cout << readBuffer <<endl;

//save to history
for(int i = 20; i > 0; i--){
   strcpy(&(historyArray[i]), &(historyArray[i-1])); //ERROR HERE//  
}

strcpy(historyArray, readBuffer); //and here but it's the same error//

But that will only fix the compiler errors, not the logical errors in the code. Your using C++ so the string solution:

vector<string> history;

cin.getline(readBuffer,512);

history.push_back(readBuffer);

Alternatively if you want one long string containing everything from readBuffer:

string history;

cin.getline(readBuffer,512);
history = history += string(readBuffer);

For example...

new299
  • 804
  • 1
  • 7
  • 17