Programming

i think theres something wrong with this code
since in the child process, it runs execl, so the child process gets replaced by ls, ls command process, so when it finishes where does the return number go to? does it go to the parent process (the fd[0] end of the pipe)? And how does Wexitstatus get the return code of the command?

Have you actually tried any of this code?

The return code goes to wait (or waitpid) as part of the status variable. The kernel actually keeps the process around in "zombie" form until you wait for it, which is how it can get the statistics even after it's dead. It's only part of the status variable which is why you need WEXITSTATUS to extract it.

Ok it sort of works now, but theres a problem, here what i did
The output is sorta correct, i gave the input command to be ls, the output was
my_prog.c
my_prog
abc
ñà·
EÉ÷·8É÷·*à·
0

The zero come up since ls was successfull, but i dont understand why the files in the directory came up line by line and not all in one line like how normally doing ls would result in, also theres some gibberish lines after the ls command results, why did that show up? Am I missing a \0 anywhere or something?

Also i tried giving a command that would fail like cd ffg, and the WEXITSTATUS number came out to be 2, so what really are the possible outcomes for WEXITSTATUS? i only know so far 0 and 2, shouldnt it be -1 instead of 2?

ls can tell the difference between a terminal and a pipe. ls | cat will do that too. In other words -- that's normal.
Remember that read() doesn't null-terminate for you -- it's a general purpose system call that can be used for anything, not just strings. So you're not only missing a NULL -- you're mistaken in that you even need NULLs.

Also, you shouldn't do printf(string); like that, since that could cause crashes if your string ends up having formatting specifiers like %s in it -- printf will try to read more parameters off the stack and crash since there aren't any. Let's use write() instead -- it needs no NULLs and can't crash in that manner.

I have no idea why you're duplicating pipes in the parent's code when you don't ever use the duplicate. Don't just throw dup2() around, only use it when you know exactly what you want to do with it why.

Furthermore, I repeat: fileno(stdin) is redundant. Use STDIN_FILENO.

Further furthermore: read first, then wait(). And keep reading until it's done, otherwise you might be throwing away tons of output.

Further further furthermore: You're still not closing your pipes before you wait(). This can cause a deadlock if the child process thinks it's waiting for you because it'll wait until the pipe closes. You could even end up waiting for yourself when you're reading from one end and still have the other end open.

Code:

int sys_new(char *comm) {
    char pipeout[512];
    int fd[2], status;
    pipe(fd);
    pid_t pid = fork();
    if (pid == 0) { 
        dup2(fd[1], STDOUT_FILENO);
        close(fd[0]); close(fd[1]);
        execl("/bin/sh", "/bin/sh", "-c", comm, NULL);
        // Need to exit() down here in case execl fails
        perror("couldn't execl");
        exit(1);
    }
    else if (pid > 0) {
        ssize_t bytes;
        close(fd[1]); // So we don't wait for ourselves forever.
     
        bytes=read(fd[0], pipeout, 512);
        while(bytes > 0) // happens when child's pipe closes.
        {
            write(STDOUT_FILENO, pipeout, bytes);            
            bytes=read(fd[0], pipeout, 512);
        }
        close(fd[0]);

        wait(&status);
    }
    else {
        perror("fork"); exit(1);
    }
    return 0;
}

Possible return values are 0 through 255, it can't return negative numbers. 0 means success, anything else means error, what the return values mean specifically depends on the application.

when I run it using ls, i get

my_prog.c
my_prog
abc
my_prog.c
my_prog
abc
Ù·Eið·8ið·ÊØ·
0

Why is this "Ù·Eið·8ið·ÊØ·" coming up when i do printf, I dont understand what you mean by adding NULL's. How can i printf it while making the output of what i want to only come up, and not that gibberish stuff?

Now I know you don't try the code I post for you, my version doesn't do that.

It's because, as stated above, read() doesn't add NULLs for you. This causes printf() to write more data than you actually read, because it doesn't know when to end.

Also as above, read() shouldn't add NULLs for you -- NULLs only make sense in C text strings, which program output might not always be. So you should write exactly the bytes you got, and exactly as many bytes as you read, nothing more, nothing less. This will work for any kind of data, and is what the code I posted above does.

i dont see where in your version are u recording the amount of bytes written, and then using that number to read that amount...

this line reads

bytes=read(fd[0], pipeout, 512);
and it reads 512 bytes which i think is more than needed, how do i know the amount written from execl?