gone
gone

Reputation: 1129

Segmentation Fault when accessing string array modified by a function in C

In my main, I define a pointer to an array of filename strings (that are to be determined):
char **commands = NULL;
I then call a function which populates the array based on argc & argv using getopt(). Before I return from the function, I print the list of files in the array and it works fine.

Back in my main, I attempt to print the same list (as confirmation) but I get a segmentation error which I can't figure out how to fix. I've included my code below.

Can someone please help me understand why commands in my main is not able to access the file list.

Cheers,
Nap

Here is the function that builds the list of file names:

int getInput (int argc, char **argv, const char *optionList, int *number, int *showEnds, char **commands, int *numberCommands) {

    char c;
    opterr = 0; // Turn off automatic error messages.
    while ( (c = getopt(argc, argv, optionList)) != -1) {
        switch (c) {
            case 'n':
                *number = TRUE;
                break;
            case 'E':
                *showEnds = TRUE;
                break;
            case '?':
                printf("Invalid switch used \"%c\"\n", optopt);
                return EXIT_FAILURE;
                break;
            default:
                printf("Input error occurred");
                return EXIT_FAILURE;
        }
    }
    printf("optind = %d,%d\n",optind, argc);

    commands = malloc(sizeof(char*) * (argc-1));

    if (optind < argc) {
        *numberCommands = (argc - optind);
        int ndx = 1;
        while (optind < argc) {
            int arraySize = (ndx+1)*sizeof(char*);
            commands = realloc (commands,arraySize);
            if (commands == NULL) {
                fprintf(stderr,"Realloc unsuccessful");
                exit(EXIT_FAILURE);
            }
            int stringSize = strlen(argv[optind])+1;
        commands[ndx] = malloc(stringSize);
            if(commands[ndx]==NULL){
                fprintf(stderr,"Malloc unsuccessful");
                exit(EXIT_FAILURE);
            }
            strcpy(commands[ndx],argv[optind]);
            ndx++;
            optind++;
        }
        printf ("Done With Input\n");
        for (int i=1; i<=*numberCommands; i++) {
            printf ("%s\n",commands[i]);
        }
    }
    return EXIT_SUCCESS;
}

Here is my main:

int main(int argc, char **argv) {

    FILE *myInput;
    FILE *myOutput;
    int result;
    const char availOptions[] = "nE";

    // Option flags
    int number = FALSE;     // (Option n) number all output lines
    int showEnds = FALSE;   // (Option E) Show Ends

    char **commands = NULL;
    int numberCommands = 0;

    // Set default to stdin/stdout.
    myInput = stdin;
    myOutput = stdout;
    if (argc > 1) {
        result = getInput(argc,argv, availOptions, &number, &showEnds, commands, &numberCommands);
        if (result != EXIT_SUCCESS) {
            exit(EXIT_FAILURE);
        }
    }
    printf ("Number of files = %d\n", numberCommands);
    for (int i=1; i<=numberCommands; i++) {
        printf ("%s\n",commands[i]);
    }

    echoInput(myInput, myOutput);
}

Upvotes: 1

Views: 529

Answers (2)

Useless
Useless

Reputation: 67723

In this function:

int getInput (/* ... */, char **commands, int *numberCommands) {

    /* ... */
    commands = malloc(sizeof(char*) * (argc-1));
}

you accept a pointer-to-pointer-to-char parameter by value. Looking at the call site, we can see that value will always be (char**)NULL. The parameter is a local variable inside your function. Even though you change it, and even though the caller happens to have a variable with the same name, you never change the caller's copy.

If you want to change the caller's copy of commands, you need to pass the address of that variable to your function:

int getInput (/* ... */, char ***commands, int *numberCommands) {

    /* ... */
    *commands = malloc(sizeof(char*) * (argc-1));
}

Now the call site looks like this:

char **commands = NULL;
int numberCommands = 0;
/* ... */
if (argc > 1) {
    result = getInput(/*...*/, &commands, &numberCommands);
    if (result != EXIT_SUCCESS) {
        exit(EXIT_FAILURE);
    }
}

note that commands and numberCommands are the two out-parameters - that is, they're the parameters where the function should change your local variable - so you're using & to pass pointers to both of them.

Upvotes: 0

Chris McGrath
Chris McGrath

Reputation: 1936

You're passing in a copy of the double-pointer

char **commands;

into the function. When you malloc() memory for it, it's not updating the pointer in your main function, only your local copy of the double-pointer. Hence, you're attempting to deference a NULL pointer in main. You could instead pass in a pointer to the double-pointer (i.e. the function would take a triple-pointer), your code would work then.

You would need to dereference the triple-pointer inside the function like so:

*commands = malloc(sizeof(char*) * (argc-1));

Alternatively, make a new double-pointer inside your function, do your work (leaving the code in your function roughly as-is) and before you return, assign the memory back into the pointer passed in from main:

*commands = local_commands;

Upvotes: 1

Related Questions