mikeizbicki / ucr-cs100

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

hw3-PATH: checking each path #495

Closed gbrlrz017 closed 9 years ago

gbrlrz017 commented 9 years ago

@mikeizbicki after I parse PATH's paths and store them in an array, I try running the command passed in by the user with each separate path on execv. Since this is all in a loop, I don't exactly have error checking each time I call execv (I want to make this part as noiseless as possible until it is absolutely necessary). I rely on one of two events happening:

A: At least one of the paths works and the rest of the code, including the for loop, do not matter because execv takes over. B: None of the paths work so my loop exits. After the loop, I have a perror which outputs my error message and the child process is exited.

This seems different from what you suggested in #494 so my question is, Is this considered bad programming style (not error checking every execv call in the loop) and will I be marked down?

mikeizbicki commented 9 years ago

This is the correct way to do it.

You will not get marked down for error checking because even though you call execv multiple times in your program, it only appears once in the code. The final perror will be displaying the error message in case ALL the execv calls fail, and that's fine.

If you have any doubt about the grading, run the checksyscalls.sh script for yourself. Whatever that says is what the grade will be.

gbrlrz017 commented 9 years ago

Awesome, thanks!

npsark commented 9 years ago

I have essentially the same setup. The only difference I have is that, after checking each of the paths in the PATH variable with execv, I also check if the input itself is the full path to the program the user wants to run. So I end up with an execv in the loop that tries each path and a second execv after that loop that attempts to run assuming the input is an absolute path. I still have only one perror though because I'm only sure there was an error if both the loop exits and the second execv is attempted. Is that a reasonable solution as far as error checking goes? The syscalls script returns -5 as the grade modifier.

mikeizbicki commented 9 years ago

@npsark That is a "correct" solution, but it is not a beautiful solution. Much prettier would be to add the folder . to your list of directories to try. Then no special cases are needed. This will make it so execv only appears in your code once.

npsark commented 9 years ago

ah that makes sense. Thanks!

gbrlrz017 commented 9 years ago

@npsark Seems like a great test case to check whether the input itself is a path. However, if one were to cd ~/rshell/bin; ls, I believe our ls program would execute instead (I could be wrong).

@mikeizbicki

  1. How should a scenario like the one I mentioned above be handled? Is it something we want?
  2. Is manually checking whether . contains the command required? By manually, I mean checking this directory when it's not a part of PATH (although I think it might always be?).
mikeizbicki commented 9 years ago

@gbrlrz017 I don't fully understand your questions, but the outline I mentioned works in all possible cases.

gbrlrz017 commented 9 years ago

@mikeizbicki I am under the impression that our ls executable (from hw1) will be present in .../rshell/bin/. When hw3 is graded, make all will create this executable. If it is present, and we change directories to bin then run ls afterward, our ls will be run instead of bash's ls. Is this something we care about?

mikeizbicki commented 9 years ago

@gbrlrz017 Now I understand. Different shells handle this case differently (in bash, it's actually something you can adjust). I don't care how you decide to handle it, but it should be documented in the README,