using strcpy for copying string into the element at index retrieved with atoi

Question

Here's the code, which is supposed to execute the first command in history when "history 1" is entered:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main (int argc, char *argv[])
{
    int i=0; int j=0; int k=0;
    char inputString[100];
    char *result=NULL;
    char delims[] = " ";
    char historyArray[100][100] = {0};
    char *tokenArray[100][100] ;

    do
    {
        j = 0;
        printf("hshell>");
        gets(inputString);
        strcpy (historyArray[k], inputString);
        k++;

        // Break the string into parts
        result = strtok(inputString, delims);

        while (result!=NULL)
        {
            //result2 = result;
            strcpy(tokenArray[j], result);
            //puts(result);
            j++;
            result= strtok(NULL, delims);                  
            //puts(tokenArray[j]);     
        }
        //j = 0;
        puts(tokenArray[0]);
        puts(tokenArray[1]);

        if (strcmp(tokenArray[0], "exit") == 0)
        {
            return 0;
        }
        else if (strcmp(tokenArray[0], "history") ==  0)
        {
           if (j>1)
           {
              strcpy (result,historyArray[atoi(tokenArray[j-1])]);

           }
           else
           {
               //print history array
               for (i=0; i<k;i++)
                   printf("%i. %s\n", i+1, historyArray[i]);
           }
        }
        else
        {
          printf("Command not found\n");
        }
    }while (1);
}

However, it crashes. When in debugging, I noticed two things: - the array (tokenArray) address is out of bounds and - Access Violation (Segmentation Fault). You can see the errors in the images below.

What am I missing? What am I doing wrong?

Answer 1

The reason why you are dealing with segmentation fault is because you are trying to copy a string into the memory that has not yet been allocated. You have defined result as a char* and just assigned NULL to it, so trying to copy string into it is wrong:

char *result = NULL;
// ...
strcpy(result, historyArray[atoi(tokenArray[j-1])]);

You need to allocate some memory, that result will point to. Then strcpy can be used to copy string into this memory. You can either use malloc to allocate it dynamically or you can define result as an temporary variable with automatic storage duration (i.e. char result[100];).

---

Also note that

char *tokenArray[100][100];

defines a two-dimensional array of pointers to char. But what you actually need in this case is an array of strings, so you need to get rid of * just like @cnicutar has pointed out.

---

And one more note:

strcpy(result,historyArray[atoi(tokenArray[j-1])]);

is quite dangerous thing to do, because when atoi fails, you are trying to access the element out of array bounds, which produces undefined behavior, thus I recommend you doing something like this:

char tokenArray[100][100] = {0};

int index;
char indexString[100] = "8";
if (sscanf(indexString, "%d", &index) == 1)     // integer successfully retrieved
{
    strcpy(tokenArray[index], "some string");
    printf("%s", tokenArray[8]);
}