Reproduce the system() function of linux

Question

My mentor says to reproduce the system() function of Linux exactly as given in the man pages. I have written the below code. Could someone review this code and provide me any suggestions for improvising. I am a beginner. Thanks :).

 #include<stdio.h>
#include<unistd.h>
#include<sys/types.h>
#include<sys/wait.h>
#include<error.h>
int my_system(const char *command);

int my_system(const char *command)
{
    int ret = 0;

    ret = execl("/bin/sh", "sh", "-c", command, (char *)NULL);
    if (ret == -1)
        error(1, 0, "error occcured in the execl() system call\n");
}


int main(int argc, char *argv[])
{
    pid_t pid;
    int ret;
    int wstatus;

    if (argc > 2)
        error(1, 0, "Too many arguments\n");
    pid = fork();
    if (pid == -1) {
        error(1, 0, "error in creating the sub-process\n");
    } else if (pid == 0) {
        ret = my_system(argv[1]);
    } else {
        wait(NULL);
    }
    return 0;
}

```

Answer 1

Enable compiler warnings and fix all of them

I immediately see that your function my_system(), which should return an int, does not contain a return statement. That means the return value is undefined. If you enable compiler warnings, the compiler will complain about such mistakes.

Don't add unnecessary forward declarations

Why add a forward declaration of my_system() right before the actual implementation? This is an unnecessary repetition of code. It's best to always try to not repeat yourself.

Read the man page again and pay attention to the details

You implemented the call to execl() exactly as written in the man page, but that is only the easy part. You have to ensure that the return value and the behavior in the presence of errors is exactly as the man page describes.

In particular, system() does not call error() when there is an error, instead the return value and the value of errno is used to convey the type of error that has occurred.

Create a test suite before making any changes to my_system()

You could change my_system() right now to fix any immediately obvious errors, but the temptation is very high that afterwards you consider the implementation to be complete. Instead, try to create a test suite first that calls my_system() in different ways to ensure that its behavior matches that given in the man page. Also, instead of making assumptions of what the required behavior is, in this case you can just check what the actual system() function does.

So such a test suite could look like:

#include <assert.h>
...
const char *commands[] = {
    "/bin/true",
    "/bin/false",
    // add more commands here
    "",
    NULL,
};

for (int i = 0; ; i++) {
    int my_ret = my_system(commands[i]);
    int my_errno = errno;
    int ret = system(commands[i]);
    assert(my_ret == ret);
    if (ret == -1)
        assert(my_errno == errno);

    if(!commands[i])
        break;
}
```