Copy argv into new array

Question

I'm getting a segmentation fault for the following code. Can somebody explain why? I would like to be able to copy the contents of argv into a new array, which I called rArray.

#include <iostream>        
using namespace std;

int main( int argc, char **argv)
{
  char **rArray;
  int numRows = argc;
  cout << "You have " << argc << " arguments:" << endl << endl;
  cout << "ARGV ARRAY" << endl;
  for (int i = 0; i < argc; i++)
  { 
    cout << argv[i] << endl;
  }
  cout << endl << endl << "COPIED ARRAY" << endl;
  for(int i; i < numRows; i++)
  {
    for (int j = 0; j < argc; j++)
      {
        rArray[i][j] = argv[i][j];
      }
  }
  for (int i = 0; i < argc; i++)
  {
    cout << "Copied array at index " << i << "is equal to " << rArray[i] << endl;;
  }
  cin.get();
}

The program outputs :

/a.out hello world
You have 3 arguments:

ARGV ARRAY
./a.out
hello
world


COPIED ARRAY
Segmentation fault: 11

Why am I getting this error? How do I fix it?

EDIT: I got a fix, changing the char **rArray to string rArray, and dynamically allocating the size from there.

Sherburn

char** rArray doesn’t allocate any memory for you, and j < argc isn’t the right condition. – Wsan

@minitech Technically char** rArray; does allocate stack space (enough to hold a pointer). – Whisker

@FrerichRaabe: Yeah, I took out “at all” because of that =P – Wsan

Answer 1

You need to allocate memory for rArray and also need to initialise the outer loop counter i.

Since the contents of argv are constant strings, you could just copy pointers to them

rArray = new char*[argc+1];
for(int i=0; i <= argc; i++) {
    rArray[i] = argv[i];
}
// use rArray
delete [] rArray;

Note that argv[argc] is guaranteed to be NULL. I've updated the loop to copy this as well (hence the unusual looking i<=argc exit condition)

If you really want to copy the content of the strings (as minitech suggests), the code becomes a bit more complicated:

rArray = new char*[argc+1];
for(int i=0; i < argc; i++) {
    int len = strlen(argv[i]) + 1;
    rArray[i] = new char[len];
    strcpy(rArray[i], argv[i]);
}
rArray[argc] = NULL;
// use rArray
for(int i=0; i < argc; i++) {
    delete [] rArray[i];
}
delete [] rArray;

Synapse

This doesn’t copy the strings themselves, does it? I think that might have been the asker’s intent. Anyways, it’s <, not <=, right? – Wsan

@minitech I've noted that the strings aren't (and needn't) be copied. <= is deliberate - I'll edit to explain why – Synapse

Yes, argv[argc] is guaranteed to be NULL, but rArray[argc] is also guaranteed to be out of bounds… – Wsan

@minitech Thanks, that was daft! I've hopefully corrected it now. – Synapse

Shouldn't it be i< argc in the top code sample of this answer too? – Flossi

@Flossi See the text immediately after the first code sample. <= is intentional to copy a pointer to the final argument passed (which happens to always be NULL) – Synapse

When I try the copy content of strings example of this answer, it gives a Bad Ptr. – Flossi

I'm using char** rArray. Is that what you're using too @Synapse ? – Flossi

I got it working! I started i at 1 and it caused the problem of it not entering the loop to copy the values into the new array. – Flossi

Shouldn't strlen(argv[i] + 1) be strlen(argv[i]) + 1? – Bioclimatology

Rather old-school, wooden solution for a C++ question, though. See e.g. @FrerichRaabe's answer for a more idiomatic approach (unless you do need the nice, spartan resource-frugality of C in your particular case). – Buzzer

Answer 2

Others pointed out various issues with your code; if you actually want to copy argv, use a std::vector of std::string objects:

#include <string>
#include <vector>

int main( int argc, char **argv ) {
    std::vector<std::string> args( argv, argv + argc );
}

Whisker

+1 - a much better solution. If you include a note about the crash-inducing problems with the current code, I'll delete my answer. – Synapse

@simonc, ahh, you meant "current code" in the question! OK, I've been desperately trying to find out how C++ is trying to kill us all again in the innocent-looking code of this answer instead. :) – Buzzer

Answer 3

One thing is that you are not initializing i

for(int i; i < numRows; i++)
        ^-- !

second thing is that rArray is not allocated

I suggest to use std::vector<std::string>, and copy all you arguments to vector, you will not need to worry about allocations/freeing memory.

Bath