CODEWITHSUNDEEP
cbashpointersstrtok
teleTele
teleTele

Reputation: 33

strtok() overwrites its source string

I'm writing a toy bash shell. My goal right now is to cycle through the environment looking for the path a specific command can be found at. Right now I am delimiting the PATH (e.g. "/home/user/bin:home/user/.local/bin:/usr/local/sbin" etc) by ":", and for each path that gives me, copying that path to the new string finalPath, and then concatenating "/cmd" to the end.

My problem is that when I try to copy the contents of path into finalPath, any changes I make to finalPath are then reflected onto path. As the code is right now, path will only be set to "home/user/bin" once, cycle through and be set to the same thing again, then the tokenizer hits "NULL" and terminates the while loop.

This suggests that path and finalPath are sharing a memory address, but since strcpy theoretically makes a new copy in memory, I must be doing something wrong with my strings and pointers.

Any idea what's causing this unexpected behavior?

Edit: this code executes just as expected when I comment out strcpy

A stripped-down version of my code is below:

int findpath(char* cmd, command_t* p_cmd) {
    char* path_var;

    path_var = getenv( "PATH" );

    char* path;
    char tempEnv[sizeof(path_var)];
    strcpy(tempEnv, path_var);
    path = strtok(tempEnv, ":");

    while(path != NULL) {
        char fullPath[1000];
        strcpy(finalPath, path);
        printf("path: %s\n", path);
        printf("finalPath: %s\n", finalPath);
        path = strtok(NULL, ":");
    }

Upvotes: 3

Views: 579

Answers (1)

cxw
cxw

Reputation: 17051

BLUEPIXY is right: the tempEnv isn't big enough for your string. Try:

char *tempEnv;
tempEnv = malloc(strlen(path_var)+1);
strcpy(tempEnv, path_var);

and at the end

free(tempEnv);

With the proviso that this is full of holes. You should be using safer string functions, e.g., as described here. For example, use strnlen to enforce set some reasonable limit on the length of path_var. Make sure path_var is NULL-terminated within that limit. Use strncpy instead of strcpy. Add the NULL after the strncpy if necessary. And a host of other rules, which I am not including here since your goal appears to be learning rather than production code. Happy hacking!

Upvotes: 3

Related Questions