1

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

transiti0nary
  • 493
  • 6
  • 25
  • `scanf("%s", args);` does not save `'\t'` nor spaces into `args`. Use `fgets()`. Suggest posting your input used. – chux - Reinstate Monica Nov 15 '16 at 21:33
  • have added input. have tried using `fgets()` but at the point where I should be prompted to enter the arguments at the command line, it just continues without prompting? – transiti0nary Nov 15 '16 at 21:39
  • As this post lacks a complete compilable code, it just makes it harder than needed to help. Adding `fgets()` likely is a problem because other code is still using `scanf()`. Strongly recommend to post a complete minimal code that shows the problem. – chux - Reinstate Monica Nov 15 '16 at 22:30
  • Sorry, there is not much extra context to the code, I've updated to include the full method. There's no other code running using scanf – transiti0nary Nov 15 '16 at 22:57

1 Answers1

4

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;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Thanks that is helpful as to how it should be correctly implemented, however `"args to %s", exe` never gets printed to console and instead program executes with no arguments with this code? This is the problem I keep having trying to use `fgets()` and `getline()`. In my original code above it would still print the prompt and execute but only recognise the first argument – transiti0nary Nov 15 '16 at 23:50
  • @transiti0nary: if you add the `fflush(stdout);`, does the prompt get printed? Are you sure you can read from standard input? I updated my answer to make failure more explicit. – chqrlie Nov 15 '16 at 23:55
  • Yeah the prompt now prints, but it still does not halt for input and just runs the exec with no arguments – transiti0nary Nov 16 '16 at 00:02
  • There must be something wrong in your environment. Can you describe how you are compiling and executing the above program? I added a simple `main` function for testing and it works fine on OS/X – chqrlie Nov 16 '16 at 00:14
  • The function is part of a larger library of functions for extracting data and performing actions on a given filepath, it would probably be simpler to try and work out what could be the cause of this sort of problem than to rake through all the other functions on here to see where the exact issue lies, I am compiling on OSX using `gcc -o fileinfo ~/fileinfolib.c ~/fileinfo.c`. `fileinfo.c` contains only a main method, which calls a library function to present file info to the user, then prompting them what to do depending on file type. If it is an exe, this method is called – transiti0nary Nov 16 '16 at 11:41
  • `gcc` is actually a wrapper on `clang` on OS/X. I suggest compiling with `-Wall -O2` or `-Weverything -O2` to get useful warnings about potential programming errors. Production code should compile without any warnings with this extra level enabled. – chqrlie Nov 16 '16 at 12:56
  • If the`execute()` function does prompt for arguments but does not read them, it is possible that `stdin` be closed redirected from a file and at the end of the file, but you should get the error message if `fgets()` returns `NULL`. There is another possibility as explained in my updated answer. – chqrlie Nov 16 '16 at 12:57
  • It seems there was something pending in the buffer as your updated answer addresses the issue. The method now works as expected, thank you! Is there any reason this while loop is fixing the problem where `fflush()` or `clearerr()` were not? – transiti0nary Nov 16 '16 at 13:20
  • @transiti0nary: `clearerr()` just clears the error condition if there was a read error, you are not likely to encounter read errors nowadays, bad sectors are quite rare now. `fflush(stdin);` invokes undefined behavior. On some environments it might clear the input buffer, but you cannot rely on this. Use the loop in my answer instead. – chqrlie Nov 16 '16 at 16:33