transiti0nary
transiti0nary

Reputation: 513

Parsing String into tokens in C - what's going wrong?

I'm trying to split a string into tokens to create an array of argument parameters. My current implementation is as follows (path is the path to the user-executable file for which optional arguments are being read):

// ARG_MAX as defined in limits.h
int execute(char *exe) {
    printf("args to %s: ", exe);

    char *args = malloc(ARG_MAX);
    scanf("%s", args);

    char *argv[ARG_MAX];

    int i = 0;
    argv[i++] = exe;

    while ((argv[i] = strsep(&args, " \t")) != NULL) {
        i++;
    }

    free(args);
    execv(exe, argv);
    return 0;
}

What's confusing me is that from my understanding of strsep this should be working as intended, and it does to an extent in that when tested it accurately allocates tokens[0] to be path, and tokens[1] to be whatever tokens_s up to the first whitespace character is.

When after a space you enter another argument, however, this is not allocated into tokens[2], and so on for subsequent arguments.

I can't seem to spot what it is I've done wrong when using strsep that isn't leading to the desired functionality?

input: exe = "/usr/bin/ps" args = "-e -l"

output: exec ps -e

Upvotes: 1

Views: 518

Answers (1)

chqrlie
chqrlie

Reputation: 144780

Multiple errors:

  • You must read the arguments with fgets() to read multiple words.

  • You must use a temporary variable for strsep() so you can pass the original pointer from malloc() back to free(), or simply use a local array.

Here is a corrected version:

#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <limits.h>

// ARG_MAX as defined in limits.h
int execute(char *exe) {
    char args[ARG_MAX + 1];

    printf("args to %s: ", exe);
    fflush(stdout);
    if (fgets(args, sizeof args, stdin)) {
        char *argv[ARG_MAX / 2];
        char *p;

        int i = 0;
        argv[i++] = exe;

        p = args;
        args[strcspn(args, "\n")] = '\0';  // strip the newline if present
        while ((argv[i] = strsep(&p, " \t")) != NULL) {
            i++;
        }

        printf("argv: ");
        for (i = 0; argv[i]; i++)
            printf(" '%s'", argv[i]);
        printf("\n");

        execv(exe, argv);
        printf("exec failed: %s\n", strerror(errno));
    } else {
        printf("cannot read input\n");
    }
    return 0;
}

int main(int argc, char *argv[]) {
    char *exe = "printf";
    if (argc > 1)
        exe = argv[1];
    return execute(exe);
}

Notes:

  • execv will not return to your program if it succeeds.

  • strsep does not collapse sequences of separators, your method will create extra arguments if you have extra spaces.

EDIT: If input is read from stdin before you get to run execute, and if such input is performed with calls to scanf(), there might be a pending newline in the stdin buffer, and fgets() would read it as an empty line. If this is the case, first flush the pending input before calling printf():

int c;
while ((c = getchar()) != EOF && c != '\n') {
    continue;
}

Upvotes: 4

Related Questions