Splitting a command line in C

Question

Is this a good C program for splitting a command line without doing any expansion on it? Don't worry too much about main() and the output -- those are for testing.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <assert.h>
void pullwhitespace(char **input);
int main (int argc, char **argv)
{
        if (argc != 2) {
                fputs("Must have exactly one command line argument\n", stderr);
                return 64;
        }
        char *input = argv[1];
        char *outstring = malloc(strlen(argv[1]) + 1);
        char *outptr = outstring;
        char a;
        pullwhitespace(&input);
NORMAL:
        /* We are in an unquoted part of the input */
        switch ((a = *input++))
        {
        case ' ' :
        case '\t':
                *outptr++ = '\0';
                pullwhitespace(&input);
                goto NORMAL;
        case '\n':
        case '\0':
                goto DONE;
        case '\\':
                if (((a = *input++)) == '\n') {
                        break;
                } else if (a) {
                        *outptr++ = a;
                        goto NORMAL;
                } else {
                        goto FAIL;
                }
        case '\'':
                goto QUOTE;
        case '"':
                goto DQUOTE;
        default:
                *outptr++ = a;
                goto NORMAL;
        }
        assert(0);
QUOTE:
        /* We are in a single quoted string */        
        while (((a = *input++)) != '\'') {
                if (a) {
                        *outptr++ = a;
                } else {
                        goto FAIL;
                }
        }
        goto NORMAL;
DQUOTE:
        /* We are in a double quoted string */
        switch ((a = *input++))
        {
        case '\0':
                goto FAIL;
        case '\\':
        {
                char b = *input++;
                switch (b)
                {
                case '\0':
                        goto FAIL;
                default:
                        *outptr++ = a;              
                case '"':              
                case '\\':
                        *outptr++ = b;
                case '\n':
                        break;
                }
        }
        goto DQUOTE;
        default:
                *outptr++ = a;
                goto DQUOTE;
        case '"':
                goto NORMAL;
        }
        assert(0);
FAIL:
        fprintf(stderr, "%s: Invalid string input\n", argv[0]);
        return 1;
        assert(0);
DONE:
        *outptr++ = '\0';
        *outptr++ = '\0';
        errno = 0;
        fwrite(outstring, 1, (size_t)(outptr - outstring), stdout);
        int error = errno;
        if(ferror(stdout)) {
                fprintf(stderr, "%s: write error: %s\n", argv[0], strerror(error));
                return 1;
        }                
        fflush(stdout);
        if ( errno != 0) {                
                fprintf(stderr, "%s: write error: %s\n", argv[0], strerror(error));
                return 1;
        }
        return 0;
}
void pullwhitespace (char **inputptr)
{
        char *input = *inputptr;
        char a;
        while (( a = *input)) {
                if (a != ' ' && a != '\t') {
                        return;
                }
                input++;
        }
}

I know that it is FULL of goto statements (more than every other program I have ever made put together) -- is that okay?

The reason for the goto statements is that this is meant as a lightweight state machine. Intended applications include parsing command line arguments or (possibly - this is far fetched) for the parser (not supporting code) to be the basis of a kernel module to parse shebang lines, allowing multiple arguments to be passed to a shebang interpreter.

Answer 1

I doubt that this code actually works. Especially pullwhitespace doesn't do what you probably intended (hint: it doesn't modify the argument, it makes a copy of the pointer and advances it). Have you tested it before submitting it for review here?

Anyway: You might want to have a look here for alternative ways to implement a state machine. You approach is similar to Remo.D's answer in the linked question but I think his macros make it more readable.

A few things I noticed:

1. When you are in NORMAL and read a white space you do this *outptr++ = '\0'; Given that strings in C are NULL terminated character sequences this seems dodgy as it will terminate the output right there unless you tell someone to read further by supplying a length. If you intend to split on white space then you should store the results in a char ** like argv.

2. Again in NORMAL:

   case '\\':
           if (((a = *input++)) == '\n') {
                   break;
           } else if (a) {
                   *outptr++ = a;
                   goto NORMAL;
           } else {
                   goto FAIL;
           }
   

   So if you find a \ followed by a newline you break the switch which is followed by an assert(0). Normally assert should be used to state invariants as "this function expects this condition to be true and it is probably a bug in the program if it is not". Specifically assert(0) should be used as "should never execute this code path". It seems weird that external user input can trigger an assert just because the program doesn't like he input. Especially since you have a FAIL state anyway.

   Also the else condition evaluation depends on the side effect of the if condition evaluation - not very nice. You should read the next character first in a separate line and then evaluate. This makes it much clearer. So this case could probably have been written as:

   case '\\':
           a = *input++;
           if (a && a != '\n') {
                  *outptr++ = a;
                  goto NORMAL;
           }
           goto FAIL;
   

3. If you are in DQUOTE and encounter a \ then you read another character from the input. However that character simply gets swallowed because in the default case you do this: *outptr++ = a; which will write the \ to the output and skip the current character (held in b).

Answer 2

Comments about pullwhitespace():

1. Code is broken. Needs work to update *inputptr.

2. Needs better name than inputptr. Maybe string_ptr. Hmm, no much better.

3. Rather than define "whitespace" as ' ' and '\t', use the C definition of whitesapce.

4. [Edit] Deleted const susggestion.

   void pullwhitespace (char **inputptr) {
     char *input = *inputptr;
     char a;
     while ((a = *input) != '\0') {
       if (!isspace((unsigned char) a)) {
         break;
       }
       input++;
     }
     *inputptr = input;
   }
   

If one accept using the C standard isspace() further simplification results in

    void pullwhitespace (char **inputptr) {
      char *input = *inputptr;
      while (isspace(*input)) input++;
      *inputptr = input;
    }

Answer 3

This parser is not that complicated, you don't need jumps. My example below ignores some corner cases, just showing the priciple. You can expand it to your needs.

#include <stdio.h>
#include <ctype.h>

int main(int argc, char **argv)
{
    const char *s, *e;

    for (e = argv[1]; *e; s = e) {
        while (isspace(*e)) e++;

        if (*e == '\"') {
            for (s = ++e; *e && *e != '\"'; e++);
            if (!*e) return 1; // missing closing quote
        } else if (*e == '\'') {
            for (s = ++e; *e && *e != '\''; e++);
            if (!*e) return 1; // missing closing quote
        } else {
            for (s = e; *e && !isspace(*e); e++);
        }
        printf("'%.*s'\n", (int)(e - s), s);

        if (*e == '\"' || *e == '\'')
            e++; // advance after closing quote
    }

    return 0;
}