0

I have a weird problem when mixing fgets and execve together.

I have a function that reads lines from a file, and executes them, either by parsing them through it's own functions, or using execve. For the sake of simplicity, my test file only has external functions:

echo whoowhee
lx
ls
dir

The second command, lx does not exist, so at that point execve should fail.

Here's the code where I am reading from the file:

  while(fgets(line, 1024, testfile) != NULL){
    if(strlen(line) < 3){continue;}
    if(line[0] == '#'){continue;}

    printf("%s%s", value("PROMPT"), line); // Emulates the prompt

    //Terminates line at right place to simulate input
    line[strlen(line)-2] = '\0';

    execute(line);

    Var *prompt = retrieveVar("PROMPT");
    sprintf(prompt->value, "< Executing script... - [%s] > $ ", value("EXITCODE"));

    lineNo++; 
  }

And here is my fork-exec block:

  if(pid == 0){ // Child

    // For loop for using all paths
    for(int i = 0; i < pathn; i++){
      args[0] = paths[i];

      // Executes command path[i] with arguments args with environment envp
      execve(paths[i], args, envp);
    }

    perror("execve");
    exit(0);
  }
  else if(pid > 0){ //Parent
    current_pid = pid;

    if(setpgid(pid, pid) != 0) perror("setpid");

    // Waits if background flag not activated.
    if(BG == 0){
      // WUNTRACED used to stop waiting when suspended
      waitpid(current_pid, &status, WUNTRACED);

        if(WIFEXITED(status)){
          setExitcode(WEXITSTATUS(status));
        }
        else if(WIFSIGNALED(status)){
          printf("Process received SIGNAL %d\n", WTERMSIG(status));
        }
    }
  }
  else{
    perror("fork()");
  }

Since execve requires absolute paths to the program, paths is an array of possible paths the program should exist in. BG states whether the process should execute in the background, value("EXITCODE") is the exitcode of the process, and execute is the function that, you guessed it, executes the line.

Now, here is the output when I run the testfile, with the code as it is:

<Welcome to Eggshell> $ echo whoowhee
whoowhee
< Executing script... - [0] > $ lx
execve: No such file or directory
< Executing script... - [0] > $ ls
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ dir
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ dir
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt

As you can see, dir is run twice.

The problems do not stop here, if I add another meaningless command to the text file:

echo whoowhee
lx
onetimesoneistwo
ls
dir

If I try to run the function, it ends up stuck in an infinite loop. Not only that, but this time it keeps on running the whole file over and over again!

However, if I remove the exit from the fork-exec bit, and rerun the function, this is what comes out:

<Welcome to Eggshell> $ echo whoowhee
whoowhee
< Executing script... - [0] > $ lx
execve: No such file or directory
< Executing script... - [0] > $ onetimesoneistwo
execve: No such file or directory
< Executing script... - [0] > $ ls
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ dir
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ ls
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ dir
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ onetimesoneistwo
execve: No such file or directory
< Executing script... - [0] > $ ls
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ dir
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ ls
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt
< Executing script... - [0] > $ dir
add-on  codecov.yml documentation  eggshell.c  LICENSE  Makefile        README.md  switch.sh      val.log
ci  createfile.txt  eggshell       eggshell.h  main.c   Makefile-Clang  src        testinput.txt

Surprisingly, it isn't an infinite loop! Sure every command after onetimesoneistwo runs 4 times, and onetimesoneistwo itself runs twice, but at least it's something.

I would guess that execve failing causes the forked child to never terminate, but then why would exit cause an infinite loop whereas not having an exit duplicate all instructions afterwards?

The funny story is that the same thing does not happen if I run the program normally, supplying the inputs myself rather than sourcing them from a file, so my guess is that something is severely wrong with either my source function, or my fork-exec code.

ENBYSS
  • 819
  • 1
  • 10
  • 22
  • replacing `exit` with `kill(SIGINT, getpid())` seems to stop the infinite loop, but the next line is still run twice. However every line afterwards runs only once. Seems like a hack of a situation, while also not even solving the entire problem... – ENBYSS May 12 '18 at 16:27
  • What exactly does the `paths` array contain? Is it the list of directory names on the current `PATH` environment variable? If so, don't you need to format the `paths[i]` and the command name into `args[0]` before you attempt to `execve()` it? – Jonathan Leffler May 12 '18 at 17:07
  • The `execve` works with actual commands. What it contains is a list of all directory names from `PATH`, with the command name appended to them. So if the command is `ls`, all paths from the `PATH` variable have `/ls` added onto them. – ENBYSS May 12 '18 at 17:08
  • 1
    That's a curious way to do it — to create a large number of formatted strings even though only one of them will be useful. Most people arrange to do the concatenation only when necessary, not just in case. It works; it just isn't the obvious way to do it. You should make sure `args[1]` is a null pointer too (assuming there are no arguments to the command; there must be a null pointer after the last argument to the command). – Jonathan Leffler May 12 '18 at 17:13
  • To be honest, I did it all at once beforehand so that I could group it all into a single function. So that when I get into the actual execution block, I'd have less unrelated code complicating it. As for the null pointer, that makes sense. I'm not sure whether it's the same, but if there are no arguments, `args[1] ` isn't even initalised at all. The debugger says it is 0x0, so I'm guessing it's a null pointer? – ENBYSS May 12 '18 at 17:17
  • You might be being lucky — maybe `args` is a file scope array, or you allocated it with `calloc()`. The onus is on you to ensure that the argument list is null terminated. – Jonathan Leffler May 12 '18 at 17:20
  • Yep, I used `calloc`. Probably forgot I did that since it's been so long. – ENBYSS May 12 '18 at 17:34

1 Answers1

0

The solution, for me at least, seemed to be to use _exit instead of exit. Using the former killed the child immediately if execve failed, rather than having a weird infinite loop effect.

Though I still have no idea why exit caused an infinite loop, and why _exit works where exit doesn't...

UPDATE: That didn't work well. What fixed the problem is using fgets in a loop before the one present, and store all the lines in a string array buffer. Then, within the next loop, I simply read the lines from the array one by one. Since exit was resetting the file pointer, the solution was to no longer use the file pointer within the loop.

ENBYSS
  • 819
  • 1
  • 10
  • 22
  • 1
    I think you're running into the problems highlighted in [Unwanted child processes being created while file reading](https://stackoverflow.com/questions/50244579/unwanted-child-processes-being-created-while-file-reading/50245276#50245276) and [Why does forking my process cause the fie to be read infinitely?](https://stackoverflow.com/questions/50110992/why-does-forking-my-process-cause-the-file-to-be-read-infinitely/50112169#50112169) It is curious that this is the third time this week the issue has arisen, but also the third time in 9+ years of my time on SO. – Jonathan Leffler May 12 '18 at 17:10
  • At least my issue wasn't as ridiculous as I thought it was, but damn, is it really that obscure? Makes sense why I couldn't find anything then... – ENBYSS May 12 '18 at 17:13
  • IMO, yes — it really is that obscure. It suddenly seems to be afflicting people. I don't recall seeing it before, even on Linux, but I'm told it has been there for a while. I've not seen it on a Mac, nor on Solaris, HP-UX, AIX (nor DG-UX, SCO, Xenix, Dynix, …) that I can recall. I don't know what's changed recently to make it suddenly appear. You should think about whether your child process should be closing the file that the parent process is reading — what is the executed command going to do with the file. Doing the close would circumvent the problems seen. – Jonathan Leffler May 12 '18 at 17:19
  • I'd say it's just a coincidence, but maybe it could just be that it took this long for a combination of `files` and `fork-exec` to happen. Happened to me because of a project to create a tiny `shell`. – ENBYSS May 12 '18 at 17:20