0

Parse line function handler:

char **parse_line(char *input) {
    char **tokens;
    char *token;
    char *seps = " \n";

    token = strtok(input, seps);
    int i = 0;
    while (token != NULL) {
        tokens[i] = token;
        i++;

        token = strtok(NULL, seps);
    }

    tokens[i] = NULL;

    return tokens;
}

Pipe function handler:

void pipes(char *input) {
    const char ch = '|';
    printf("%s", input);

    char *c;

    if (strchr(input, ch) == NULL) {
        printf("no | found\n");
        return;
    }

    printf("%s\n", input);

    char *p = strtok(input, "|");

    int i = 0;
    char *array0;
    char *array1;

    while (p != NULL) {
        if (i == 0)
            array0 = p;
        else
            array1 = p;
        i++;
        printf("p: %s\n", p);

        p = strtok( NULL, "|" );
    }

    printf("opening\n");

    parse_line(array1);
}

Main

int main(int argc, const char *argv[]) {
    while (1) {
        printf("> ");
        //read line
        char input[100];
        fgets(input, sizeof(input), stdin);

        pipes(input);
    }
    ...

gdb output:

(gdb) cat scores | grep villanova
Undefined catch command: "scores | grep villanova".  Try "help catch".
(gdb) run
Starting program: ******* 
> cat scores | grep villanova
cat scores | grep villanova
cat scores | grep villanova

while
while
opening
 grep villanova


Program received signal SIGSEGV, Segmentation fault.
0x00007fffffffddf2 in ?? ()
(gdb) x/s 0x00007fffffffddf2
0x7fffffffddf2: "villanova"
(gdb) p $_siginfo._sifields._sigfault.si_addr
$1 = (void *) 0x7fffffffddf2
(gdb) q

When stepping through with gdb it seg faults after it reaches the end of the pipes function. Anyone have any idea why/ how I can find out why and fix it.

I'm trying to get better at debugging but this has stumped me and I'd appreciate any help I can get :)

chqrlie
  • 131,814
  • 10
  • 121
  • 189
hello
  • 13
  • 5
  • 7
    In the `parse_line` function you have a pointer `tokens`, but you never make it point anywhere? Uninitialized local non-static variables (a.k.a. automatic variables) are really not initialized. Their contents will be *indeterminate*. Dereferencing an uninitialized pointer (which you do with `tokens[i]`) leads to *undefined behavior*. – Some programmer dude Sep 12 '17 at 07:08
  • 3
    compile your code with all warnings & debug info (`gcc -Wall -Wextra -g`). Improve it to get no warnings. Learn also to use `strace`. Read http://advancedlinuxprogramming.com/ and avoid [undefined behavior](https://en.wikipedia.org/wiki/Undefined_behavior). Better define some data structures (use `struct` and pointers to them). Study for inspiration the source code of some existing shell (e.g. `sash`, `bash`, ...) – Basile Starynkevitch Sep 12 '17 at 07:11
  • You anyway ignore the return value of `parse_line`, what's the point returning? Following that, it eliminates the need of having `tokens` altogether. – Sourav Ghosh Sep 12 '17 at 07:19
  • Also read carefully the documentation of every function that you are using. – Basile Starynkevitch Sep 12 '17 at 07:20
  • 1
    Adding to Basile's list of compiler flags, `-Werror` - i.e treat all those warnings you just turned up as errors, and don't build the program until they're fixed. Warnings are nearly *always* a sign of something dreadfully wrong and as-such, you should consider them flat-out errors. So, tell your compiler to treat them as such. – WhozCraig Sep 12 '17 at 07:25

1 Answers1

3

The function parse_line does not allocate an array for tokens to point to. The code has undefined behavior.

Here is a simple corrected version:

char **parse_line(char *input) {
    size_t i = 0;
    char **tokens = malloc(sizeof(*tokens));
    char *token;
    const char *seps = " \n";
    if (tokens == NULL)
        return NULL;

    token = strtok(input, seps);
    while (token != NULL) {
        char **newp = realloc(tokens, (i + 2) * sizeof(*newp));
        if (newp == NULL) {
            free(tokens);
            return NULL;
        }
        tokens = newp;
        tokens[i++] = token;
        token = strtok(NULL, seps);
    }
    tokens[i] = NULL;

    return tokens;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189