Different execution flow using read() and fgets() in C

Question

I have a sample program that takes in an input from the terminal and executes it in a cloned child in a subshell.

#define _GNU_SOURCE
#include <stdlib.h>
#include <sys/wait.h>
#include <sched.h>
#include <unistd.h>
#include <string.h>
#include <signal.h>

int clone_function(void *arg) {
  execl("/bin/sh", "sh", "-c", (char *)arg, (char *)NULL);
}

int main() {
  while (1) {
    char data[512] = {'\0'};
    int n = read(0, data, sizeof(data));
    // fgets(data, 512, stdin);
    // int n = strlen(data);
    if ((strcmp(data, "exit\n") != 0) && n > 1) {
      char *line;
      char *lines = strdup(data);
      while ((line = strsep(&lines, "\n")) != NULL && strcmp(line, "") != 0) {
        void *clone_process_stack = malloc(8192);
        void *stack_top = clone_process_stack + 8192;
        int clone_flags = CLONE_VFORK | CLONE_FS;
        clone(clone_function, stack_top, clone_flags | SIGCHLD, (void *)line);
        int status;
        wait(&status);
        free(clone_process_stack);
      }
    } else {
      exit(0);
    }
  }
  return 0;
}

The above code works in an older Linux system (with minimal RAM( but not in a newer one. Not works means that if I type a simple command like "ls" I don't see the output on the console. But with the older system I see it.

Also, if I run the same code on gdb in debugger mode then I see the output printed onto the console in the newer system as well.

In addition, if I use fgets() instead of read() it works as expected in both systems without an issue.

I have been trying to understand the behavior and I couldn't figure it out. I tried doing an strace. The difference I see is that the wait() return has the output of the ls program in the cases it works and nothing for the cases it does not work.

Only thing I can think of is that read(), since its not a library function has undefined behavior across systems. But I can't agree as to how its affecting the output.

Can someone point me out to why I might be observing this behavior?

EDIT

The code is compiled as:

gcc test.c -o test

strace when it's not working as expected is shown below

strace when it's working as expected (only difference is I added a printf("%d\n", n); following the call for read())

Thank you
Shabir

Answer 1

There are multiple problems in your code:

• a successful read system call can return any non zero number between 1 and the buffer size depending on the type of handle and available input. It does not stop at newlines like fgets(), so you might get line fragments, multiple lines, or multiple lines and a line fragment.
• furthermore, if read fills the whole buffer, as it might when reading from a regular file, there is no trailing null terminator, so passing the buffer to string functions has undefined behavior.
• the test if ((strcmp(data, "exit\n") != 0) && n > 1) { is performed in the wrong order: first test if read was successful, and only then test the buffer contents.
• you do not set the null terminator after the last byte read by read, relying on buffer initialization, which is wasteful and insufficient if read fills the whole buffer. Instead you should make data one byte longer then the read size argument, and set data[n] = '\0'; if n > 0.

Here are ways to fix the code:

• using fgets(), you can remove the line splitting code: just remove initial and trailing white space, ignore empty and comment lines, clone and execute the commands.
• using read(), you could just read one byte at a time, collect these into the buffer until you have a complete line, null terminate the buffer and use the same rudimentary parser as above. This approach mimics fgets(), by-passing the buffering performed by the standard streams: it is quite inefficient but avoids reading from handle 0 past the end of the line, thus leaving pending input available for the child process to read.

Answer 2

It looks like 8192 is simply too small a value for stack size on a modern system. execl needs more than that, so you are hitting a stack overflow. Increase the value to 32768 or so and everything should start working again.