user1373475
user1373475

Reputation: 33

Crashing in a for loop

I'm trying to write to a text file. I can write fine when I don't use my for loop, but when I implement it to write all of my array to the file it crashes. Here's my code:

void writeFile(void)
{
  char *fileName[30];
  cout << "enter a filename";
  cin >> *fileName;
  ofstream myfile;
  myfile.open (*fileName);
  int p;

  for(p = 0; p <= i; p++)
    {
      myfile << right << setw(4) << setfill('0') << packet[i].getSource() <<  ":";
      myfile << right << setw(4) << setfill('0') << packet[i].getDest() <<  ":";
      myfile << right << setw(4) << setfill('0') << packet[i].getType() <<  ":";
      myfile << right << setw(4) << setfill('0') << packet[i].getPort() <<  endl;
    }

Any ideas where I'm going wrong?

Upvotes: 1

Views: 200

Answers (3)

EdChum
EdChum

Reputation: 394051

There is another thing slightly dodgy here:

for(p = 0; p <= i; p++)

you probably want

for(p = 0; p < i; p++)

so that you don't try to dereference off the end of your array

probably better to write

for (int p = 0; p != i; ++p)

This is the recommended form according to Moo and Koenig: http://www.drdobbs.com/cpp/184402072

I would also not use char * to read from a cin, use std::string to store your string and inputs, you do not need to new the memory if it is not needed outside the scope of your main writeFile function. Strings support dynamic resizing also so you don't need to initialise it to any size, here is the first example I googled to help you understand

Upvotes: 2

Uflex
Uflex

Reputation: 1426

Why are you using the "C way" to store your file name? And you're using it the wrong way: char**. It would be easier to just declare:

std::string fileName;
while(!std::cin >> fileName);
ofstream myfile(fileName.c_str());

You also are using i inside of your loop but are iterating over p, I think that's not what you want to do ...

Upvotes: 1

Kerrek SB
Kerrek SB

Reputation: 477060

fileName is an array of 30 uninitialized pointers to char. *fileName is the same as filename[0], which is an uninitialized pointer to a char. You cannot use this pointer for anything but assigning it a valid value. You are not doing that, though, and instead you're trying to read data to it, with predictably catastrophic consequences.

In short, you shouldn't be using any pointers in C++ at all, and instead use an std::string for your situation:

std::string fileName;
if (!(std::cin >> fileName)) { /* I/O error, die */ }
// ...

(Perhaps what you meant to do is to make fileName an array of 30 chars: char fileName[30];. But don't do that. Even though it might work, it's very terrible.)

Upvotes: 3

Related Questions