Odd array behaviour with strtok and fgets

Question

I'm working on a program that acts as a shell interpreter that reads commands with arguments and creates a child that executes the command with execvp(). I'm stuck on doing some string manipulation to collect a char array *args[], particularly with using fgets and strtok.

Here's an MCVE of my code.

#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>

#define MAX_LINE 80

int main(void){
    //initialize variables
    char *args[MAX_LINE/2 + 1];
    char input[MAX_LINE];
    //char input[MAX_LINE] = "some sentence unknown"; // <-- this line works fine..

    int counter = 0;
    printf("COMMANDER>");
    fflush(stdout);

    //receive input
    fgets(input,MAX_LINE,stdin);

    //parse input
    char *parser;
    parser = strtok(input," \r\t");

    //parse line
    while(parser != NULL){
                args[counter] = parser;
        counter++;  
        parser = strtok(NULL," ");
    }

    //print results
    int i = 0;
    for(i = 0; i < counter + 1;i++){
        printf("1");
        printf(" - %d: %s\n",i,args[i]);    
    } 

    return 0;
}

The problem here is the ouput. When I try to run this, I get the following output:

COMMANDER>some sentence unknown
1 - 0: some
1 - 1: sentence
1 - 2: unknown

1 - 3: (null)

My issue is that empty space. I can't tell where it's coming from and no matter what I do it appears.

From what I can tell it may be a \n character at the end of the string, or something else, but passing this into execvp as execvp(args[0],args) creates an error, as it interprets this blank line as an argument of " ".

There is a line I've commented out that's just a string assignment at the start of main. If this assignment is used instead of fgets the program works and I get the desired input:

COMMANDER>some sentence unknown
1 - 0: some
1 - 1: sentence
1 - 2: unknown
1 - 3: (null)

Thanks for reading. I'm a bit rusty on my C, so I kept at this on my own for a few hours and still couldn't find a solution.

See https://stackoverflow.com/questions/2693776/removing-trailing-newline-character-from-fgets-input — pmg, Sep 30 '18 at 15:50
I suggest changing the separator string to `" \n\r\t"` escpecially in the second `strtok` you omitted to separate on *any* control characters. — Weather Vane, Sep 30 '18 at 15:53

Some programmer dude · Accepted Answer · 2018-09-30T15:54:52.387

If you read e.g. this fgets reference you will see that it says

Parsing stops if end-of-file occurs or a newline character is found, in which case str will contain that newline character.

_{[Emphasis mine]}

What you're seeing as "that empty space" is the newline that fgets added at the end of the string.

There is however a much worse problem in your code.

With

char *args[MAX_LINE/2 + 1];

you define an array of pointers, but you leave this array uninitialized. In C uninitialized local (and non-static) variables really are uninitialized. Their contents will be indeterminate and seem almost random.

More specifically, that you happen to get a null pointer at args[counter] is just pure luck.

Attempting to use this pointer in any way without initialization will lead to undefined behavior.

The simple solution is to explicit initialize the array to be full of null pointers:

char *args[MAX_LINE/2 + 1] = { NULL };

The above will "zero-initialize" all elements, which for pointers mean they will be NULL.

Thanks! that makes a lot of sense. The null pointer at the end was originally assigned as a statement but I deleted it just before asking this question. — Howard P, Sep 30 '18 at 15:54

score 1 · Answer 2 · answered Sep 30 '18 at 16:18

The line read by fgets() contains a trailing newline character '\n'. You must include it in the list of characters recognized by strtok() as delimiters. Furthermore, you must pass the same list to both calls.

You also forget to set the entry at offset count to NULL. Local objects with automatic storage are uninitialized. Note also that you should not pass a null pointer to printf for the %s conversion specifier: it has undefined behavior although some implementations test the argument and output a specific string.

Here is a corrected version:

#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>

#define MAX_LINE 80

int main(void){
    char *args[MAX_LINE/2 + 1];
    char input[MAX_LINE];
    int counter;

    printf("COMMANDER>");
    fflush(stdout);

    //receive input
    if (fgets(input, sizeof input, stdin)) {
        //parse input
        char *parser;
        parser = strtok(input, " \f\n\r\t");

        //parse line
        count = 0;
        while (parser != NULL) {
            args[counter] = parser;
            counter++;  
            parser = strtok(NULL, " \f\n\r\t");
        }
        args[counter] = NULL;

        //print results
        int i;
        for (i = 0; i <= counter; i++) {
            printf("1 - %d: %s\n", i, args[i] ? args[i] : "(null)");
        } 
    }
    return 0;
}

Odd array behaviour with strtok and fgets

2 Answers2