Execve not working as expected

Question

I am writing a basic shell in c that will allow me to execute simple commands(I am not asked to check for optional arguments) like "ls" in a server (localhost). The program must be able to handle multiple clients.

I have done everything up to the part I have to execute the command with execve() (I MUST USE THIS FUNCION). I've found that execve() returns -1 on failure and that it doesn't return anything on success so I am making a fork() to execute the command in that process.

Now, to the problem. How do I know if execve() executed successfully? I can't seem to find the problem, my code always returns "OK" to the client. "csapp.h" it's just a source file containing wrappers around some functions.

#include "csapp.h"

void echo(int connfd, pid_t pid);

int main(int argc, char **argv)
{
    int listenfd, connfd;
    unsigned int clientlen;
    struct sockaddr_in clientaddr;
    struct hostent *hp;
    char *haddrp, *port;
    pid_t pid;

    if (argc != 2) {
        fprintf(stderr, "usage: %s <port>\n", argv[0]);
        exit(0);
    }
    port = argv[1];

    listenfd = Open_listenfd(port);
    while (1) {
        clientlen = sizeof(clientaddr);
        while(1){
            connfd = Accept(listenfd, (SA *)&clientaddr, &clientlen);
            if((pid=Fork())==-1){
                Close(connfd);
            }
            if(pid > 0){
                break;
            }
        }

        /* Determine the domain name and IP address of the client */
        hp = Gethostbyaddr((const char *)&clientaddr.sin_addr.s_addr,
                    sizeof(clientaddr.sin_addr.s_addr), AF_INET);
        haddrp = inet_ntoa(clientaddr.sin_addr);
        printf("server connected to %s (%s)\n", hp->h_name, haddrp);

        echo(connfd, pid);
        Close(connfd);
    }
    exit(0);
}

void trim(char *string){
    string[strlen(string)-1]=0;

}

char* concat(const char *s1, const char *s2)
{
    char *result = malloc(strlen(s1) + strlen(s2) + 1); // +1 for the null-terminator
    // in real code you would check for errors in malloc here
    strcpy(result, s1);
    strcat(result, s2);
    return result;
}

void echo(int connfd, pid_t pid)
{
    size_t n;
    char buf[MAXLINE];
    rio_t rio;

    char *args[2];

    args[1] = NULL;

    Rio_readinitb(&rio, connfd);
    while((n = Rio_readlineb(&rio, buf, MAXLINE)) != 0) {
        trim(buf);
        args[0] = concat("/bin/", buf);
        printf("server received %lu bytes\n", n);
        printf("Command: %s\n",buf);
        pid_t execPID;
        int status;
        if((execPID = fork()) > pid){
            execve(args[0],args,NULL);
        }else{

            wait(&status);
            if(WIFEXITED(status)){
                if (WEXITSTATUS(status) == 0){
                    printf("status: %d\n", status);
                    printf("WIFEXITED: %d\n", WIFEXITED(status));
                    printf("WEXITSTATUS: %d\n", WEXITSTATUS(status));
                        Rio_writen(connfd, "OK\n", 3); 
                }
                else
                        Rio_writen(connfd, "ERROR\n", 6);

            }
        }
        /*if(status == -1){
                Rio_writen(connfd, "ERROR\n", 6);

        }
            else{
                Rio_writen(connfd, "OK\n", 3);
                printf("%d\n", status);
            }*/


    }

}

Output for "m" and "ls" sent by the client:

server received 2 bytes
Command: m
status: 0
WIFEXITED: 1
WEXITSTATUS: 0
server received 3 bytes
Command: ls
status: 0
WIFEXITED: 1
WEXITSTATUS: 0
Makefile   client    csapp.c  csapp.o  server.c
README.md  client.c  csapp.h  server

I'd really appreciate some help, I've been stuck at this for the past 14 hours.

Answer 1

As @Stargateur said in the comments below the question, I was doing wrong the comparison of execPID. Basically, pid is positive when it's the parent process and zero when it's the child process, so changing if ((execPID = fork()) > pid) to if ((execPID = fork()) == 0) actually solves the problem.

Answer 2

How do I know if execve() executed successfully?

In the first place, as you've already observed, the server knows that the execve() call itself has failed if it returns at all.

In the second place, after the child process terminates, wait() or waitpid() can tell you what its exit status was. You seem to have that worked out, too.

So when you go on to say,

I can't seem to find the problem, my code always returns "OK" to the client.

, that leads me to believe that your actual question is more along the lines of "how do I get the command's output?" Answer: via the child's standard output and (possibly) standard error streams. How else?

By default, the child inherits its standard input, standard output, and standard error from its parent, but you can give it different ones by calling dup2() in the child process before execve(). You can do a variety of fairly sophisticated things with that by connecting the child's streams to pipe()s that the parent or some other process can read, but the simplest thing to do would be to use dup2() to simply direct the child's output and error streams to the socket:

// standard output:
if (dup2(connfd, 1) == -1) { /* handle error */ }

// standard error:
if (dup2(connfd, 2) == -1) { /* handle error */ }

Then the command's output, if any, will be sent directly over the wire to the client.

Answer 3

this line:

{
    execve(args[0],args,NULL);
}

should be changed to:

{
    execve( args[0],args,NULL );
    perror( "execve failed" );
    exit( EXIT_FAILURE );
}

Then, the code does not continue to execute if the call to execve() fails AND if it fails, tells you why.

Note: both exit() and EXIT_FAILURE are from the header file: stdlib.h