mikeizbicki / ucr-cs100

open source software construction course
Other
485 stars 407 forks source link

Trouble with path and execv #87

Closed DanTheTingler closed 10 years ago

DanTheTingler commented 10 years ago

So I've been able to parse the input I get back from getenv("PATH") and use it in execv. However, when I enter a command, it works the first time, and from that point on nothing outputs from the commands I use after. For example, if I call ls, it will work, but if I try to call ls again, or ls -l, nothing outputs to the screen.

Here is an example (go here is something I'm outputting in my for loop to try and error check):

tingles@Tingles$ ls
goes here
goes here
goes here
goes here
goes here
goes here
a.out  ls.cpp  result  rshell.cpp
tingles@Tingles$ ls
goes here

Here are the functions I'm using the get the path and parse it for my use:

char *allpaths(const char *name)
{
    char *pathing = getenv(name);
    return pathing;
}

char **argmodifier(char *paths)
{
    int i = 0;

    char **arg;
    arg = new char* [2048];

    char *token = strtok(paths, " :"); //parse input using strtok
    while (token != NULL)
    {
        arg[i] = token; //put parsed commands into arg
        i++; //incrememnt to the next part of arg
        token = strtok(NULL, " :"); //continue parsing
    }

    arg[i] = NULL; //set the end of arg to NULL to avoid seg fault

    return arg;
    delete[] arg;
}

char *finalpathmod(char *path, char *command)
{
    char *finalpath = new char[1024];
    strcpy(finalpath, path);
    strcat(finalpath, "/");
    strcat(finalpath, command);

    return finalpath;

}

And here is how I'm calling exec and the three functions above:

void reg(char str[], char *argv[])
{
    char *pathing = allpaths("PATH");
    char **westbrook = argmodifier(pathing);
    char *kd = new char[1024];

    bool amper = false;
    int i = 0;

    char **arg;
    arg = new char* [1024];

    char *token = strtok(str, " "); //parse input using strtok
    while (token != NULL)
    {
        arg[i] = token; //put parsed commands into arg
        i++; //incrememnt to the next part of arg
        token = strtok(NULL, " "); //continue parsing
    }

    arg[i] = NULL; //set the end of arg to NULL to avoid seg fault

    //search for the various needed elements
    for(int d = 0; d < i; d++)
    {
        if(*arg[d] == '&') //if & is found
        {
            amper = true; //set bool to true
        }
    }

    pid_t pid = fork(); //error checking is in the else

    if(pid > 0) //parent
    {
        if(amper == false) //if there was no &, then wait for child process
        {
            wait(0);
        }
    }

    else if(pid == 0) //child process, which means we want to run exec
    {
        for(int f = 0; westbrook[f] != NULL; f++)
        {
            kd = finalpathmod(westbrook[f], arg[0]);
            cout << "goes here" << endl;
            execv(kd, arg);
        }
        exit(1);
    }

    else //error check for fork: fork has failed, output error message
    {
        perror("fork has failed: ");
        exit(1);
    }

    delete[] westbrook;
    delete[] arg;
}
DanTheTingler commented 10 years ago

I modified my for loop inside the child function a bit, but it's still not working. Here's what I did:

for(int f = 0; westbrook[f] != NULL; f++)
        {
            kd = finalpathmod(westbrook[f], arg[0]);
            cout << "goes here" << endl;
            if(execv(kd, arg) == -1) //if execv wasn't successful then keep going
            {
                continue;
            }
            else //if execv is successful, no need to check the other paths anymore
            {
                break;
            }
        }
bmars003 commented 10 years ago

There is one issue with your code that I see that is unrelated to the question you are asking. But from the code you provided is that you parse the environmental variable PATH, then run execv on all off them till you hit a match. What I would suggest first is see if the file/executable exists before running execv on it.

TLDR: Try only running exec of commands that you know exist in the PATH environmental variable.

DanTheTingler commented 10 years ago

Shouldn't that not matter since exec checks the commands anyway?

mikeizbicki commented 10 years ago

You should carefully compare the code you have to the code of your previous submissions. You should not have to modify any of the code that deals with forking or other processes in this assignment. But your error makes it sound like you have. So go through a line-by-line comparison (maybe with the diff command) and see what has changed.

DanTheTingler commented 10 years ago

I haven't modified any code besides using evecv instead of execvp. All I've done is added in the functions/code needed to parse getenv("PATH") and put it together with what the user has. That, and I also added the for loop to check everything in PATH.

mikeizbicki commented 10 years ago

I don't believe you :)

Again, my advice is to use the diff command (or git diff) to find the changes you've made. Then add them back in one at a time until you can reproduce the error. At that point you know where the bug is.

DanTheTingler commented 10 years ago

Hmm, so there isn't anything blantantly wrong with my code that might be causing this problem?

mikeizbicki commented 10 years ago

I'm not telling :) It wouldn't help you at all if I just told you. This is a debugging technique that you need to master. If you have any questions on the procedure, I'd be happy to answer about that.

DanTheTingler commented 10 years ago

Can't I at least get a hint?! :( I feel like it has something to do with the way I'm calling exec/looping through the path, but I'm not too sure what's wrong with it. I feel like the 3 functions I wrote to parse the output from getenv are good.

mikeizbicki commented 10 years ago

I already gave you a hint. The process I outlined is guaranteed to find the bug. If you have specific questions about that, I'd be happy to answer them.

DanTheTingler commented 10 years ago

Okay mike, I found the line that causes the error! It's this line:

char **westbrook = argmodifier(pathing);

which I'm assuming means there's something wrong with my argmodifier?

To elaborate, when I commented out that line, I was able to use execvp again (which was how my program WAS written). When that line was in there however, my old code wouldn't work.

DanTheTingler commented 10 years ago

So I guess my specific question is, what exactly is wrong with my argmodifier function?

DanTheTingler commented 10 years ago

So I've been trying to fix my argmodifier function this weekend without much luck, could I get a couple of hints? I used your debugging methods Mike, and they all pointed to that function and specific line of code that I posted above.

mikeizbicki commented 10 years ago

If execv is not doing what you think it should, then the inputs are probably not what you think they are. Inspect them and verify they are correct.

DanTheTingler commented 10 years ago

The thing is, execv does what I think/want it to do one time, then breaks for some reason after that.

DanTheTingler commented 10 years ago

By the way, can I post this exact question, along with my code, on stack overflow to see if I can get any third-party help? Or am I not allowed to post my code anywhere online.

mikeizbicki commented 10 years ago

I strongly encourage you to use stackoverflow.

DanTheTingler commented 10 years ago

Never mind, I found the error myself! Turns out I had to convert pathing, which was a char pointer, to a char array before passing it into argmodifier. Thanks for the help!