3 Replies - 1632 Views - Last Post: 07 March 2013 - 08:56 PM Rate Topic: -----

#1 fredszky  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 7
  • Joined: 28-January 13

execvp() and my program gets stuck, send() and it works.

Posted 06 March 2013 - 01:07 PM

Server code part:
                    void spawn (char* program, char** arg_list)  // Executing the clients command
                    {

                        printf("\n\nprogram = %s\narg_list = %s\n\str1=%s!!!\nstr2=%s\n\n",program,arg_list,str1,str2);

                        dup2(s2, STDOUT_FILENO);  //Redericting information to socket
                        dup2(s2, STDERR_FILENO);
                        execvp (program, arg_list);abort ();    // Executing the clients command - After this, program gets stuck, why?

                        /*
                        if (send(s2, "This works multiple times", 27, 0) == -1) {
                        perror("send");
                        exit(1);
                        }  */
                    }


So, im making a server to handle multiple clients. The clients sends simple commands like "ls -l" or "mkdir test" and the server executes and send information back to the client.

When i connect my client, i can get the output from the server to my client, one time, but then the server program seems to get stuck.
If i use a test string with send(), it will work multiple times as it should be.

Why does my program gets stuck using execvp?


Whole server code:

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>

#define SOCK_PATH "socket"


int main( int argc, char *argv[] )
{

    int s, s2, t, len;
    struct sockaddr_un local, remote;
    char str[1000];
    char* arg_list[2220];
    char str1[1000], str2[1000];


    int z=0;
    for(z=0;z<99;z++){
    str1[z] = NULL;
    str2[z] = NULL;
    }

    for(;;)/>/>/>/>/>{


        if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
            perror("socket");
            exit(1);
        }

        local.sun_family = AF_UNIX;
        strcpy(local.sun_path, SOCK_PATH);
        unlink(local.sun_path);
        len = strlen(local.sun_path) + sizeof(local.sun_family);
        if (bind(s, (struct sockaddr *)&local, len) == -1) {
            perror("bind");
            exit(1);
        }

        if (listen(s, 5) == -1) {
            perror("listen");
            exit(1);
        }

        printf("Waiting for connections\n");

        t = sizeof(remote);
        if ((s2 = accept(s, (struct sockaddr *)&remote, &t)) == -1) {   // Server waits for client to connect
            perror("accept");
            exit(1);
        }


        pid_t childPID;

        childPID = fork();

        if(childPID >= 0) // Fork was successful
        {
            if(childPID == 0) // Child process
            {
                printf("\I am child\n");

                close(s);

                printf("Connected.\n");

                int        active = 0;
                int	done = 0;
                int sent = 0;
                do{

                    int n = recv(s2, str, 1000, 0);
                    if(n <= 0) {
                        if (n < 0) perror("recv");
                            done = 1;
                    }

                    active = 1;

                    str[strcspn ( str, "\n" )] = '\0';  // String stuff below ***

                    strcpy(str1,str);

                    int i=0,k=0;

                    while (str1[i] != ' ')
                        i++;

                    while (str1[i] != '\0'){
                        str2[k] = str1[i+1];
                        k++;
                        i++;
                    }

                    i=0;

                    while (str1[i] != ' ')
                    i++;

                    str1[i] = '\0'; // String stuff above ***


                    void spawn (char* program, char** arg_list)  // Executing the clients command
                    {

                        printf("\n\nprogram = %s\narg_list = %s\n\str1=%s!!!\nstr2=%s\n\n",program,arg_list,str1,str2);

                        dup2(s2, STDOUT_FILENO);  //Redericting information to socket
                        dup2(s2, STDERR_FILENO);
                        execvp (program, arg_list);abort ();    // Executing the clients command - Program gets stuck, why?

                        /*
                        if (send(s2, "This works multiple times", 27, 0) == -1) {
                        perror("send");
                        exit(1);
                        }  */
                    }


                    char command[2222];


                    char * tmp;

                    i=0;

                    tmp = strtok (command," ");
                    arg_list[0] = tmp;

                    while (tmp != NULL)
                    {
                        tmp = strtok (NULL, " ");
                        arg_list[i+1] = tmp;
                        i++;
                    }

                    char* arg1[] = {
                        str1,
                        NULL
                    };
                    char* arg2[] = {
                        str1,
                        str2,
                        NULL
                    };


                    if (!done && sent == 0 && active == 1){
                        if(str2[0] >= '+' && str2[0] <= 'DEL'){     // Ugly solution to make commands work.. .sometimes.
                            if(str2[1] >= '+' && str2[1] <= 'DEL')

                                spawn (str1, arg2);}
                            else
                                spawn (str1, arg1);
                    }

                    sent = 0;
                    for(z=0;z<99;z++){
                        str1[z] = NULL;
                        str2[z] = NULL;
                    }

                }while (!done);


            }
            else //Parent process
            {
                printf("\nI'll just get back to my listening state - the (;;)/>/>/>/>/> loop \n");
            }

        }
        else // Fork failed
        {
            printf("\nFork failed, quitting!\n");
            return 1;
        }
    }
    return 0;
}



Whole client code:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>

#define SOCK_PATH "socket"

int main(void)
{
    int s, t, len,i=0;
    struct sockaddr_un remote;
    char str[10000];

    if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
        perror("socket");
        exit(1);
    }

    printf("Trying to connect...\n");

    remote.sun_family = AF_UNIX;
    strcpy(remote.sun_path, SOCK_PATH);
    len = strlen(remote.sun_path) + sizeof(remote.sun_family);
    if (connect(s, (struct sockaddr *)&remote, len) == -1) {
        perror("connect");
        exit(1);
    }

    printf("Connected.\n");

    while(printf("> "), fgets(str, 10000, stdin), !feof(stdin)) {


        if (send(s, str, strlen(str), 0) == -1) {
            perror("send");
            exit(1);
        }

        if ((t=recv(s, str, 10000, 0)) > 0) {
            str[t] = '\0';
            printf("echo> %s", str);

            for(i=0;i<9999;i++)
            str[i]=NULL;

        } else {
            if (t < 0) perror("recv");
            else printf("Connection with server terminated.\n");
            exit(1);
        }
    }

    close(s);

    return 0;
}




Is This A Good Question/Topic? 0
  • +

Replies To: execvp() and my program gets stuck, send() and it works.

#2 Salem_c  Icon User is offline

  • void main'ers are DOOMED
  • member icon

Reputation: 2130
  • View blog
  • Posts: 4,196
  • Joined: 30-May 10

Re: execvp() and my program gets stuck, send() and it works.

Posted 07 March 2013 - 12:51 AM

Ugh, where to begin.

1. Your server main is far too long (and deeply nested). As a rough rule of thumb, if you can't see the whole function on screen at the same time (namely both it's opening and closing braces), then it's probably time to think about splitting things up.

2. Standard C doesn't support nested functions, so move spawn() outside main.

3. Too many magic numbers.
    char str[1000];
    char* arg_list[2220];
    char str1[1000], str2[1000];

Have something like
#define MAX_STRING_LEN 1000
and use it in all the appropriate places.

4. Pointless code
    for(z=0;z<99;z++){
    str1[z] = NULL;
    str2[z] = NULL;
    }

Not only do you get the length wrong, NULL is semantically associated with pointers, and not the nul char, otherwise written as '\0'
Also, such code is pointless if things are being done properly elsewhere (more on that later).

5. First big function split-off
        if(childPID >= 0) // Fork was successful
        {
            if(childPID == 0) // Child process
            {
                printf("\I am child\n");
                close(s);
                handleConnection(s2);
            }
            else //Parent process
            {
                printf("\nI'll just get back to my listening state - the (;;)/>/>/>/>/>/>/> loop \n");
            }
        }
        else // Fork failed
        {
            printf("\nFork failed, quitting!\n");
            return 1;
        }

Move about a hundred lines of code into the handleConnection() function.

6. Handling recv properly.
                    int n = recv(s2, str, 1000, 0);
                    if(n <= 0) {
                        if (n < 0) perror("recv");
                            done = 1;
                    }

                    active = 1;

                    str[strcspn ( str, "\n" )] = '\0';  // String stuff below ***


First, use the symbolic array length (or better yet, sizeof) to indicate how much buffer space recv has.
Second (important), you need to understand that recv does NOT make strings out of everything.
Third (very important), you need to deal with message fragmentation. This is probably not an issue for AF_UNIX sockets, but when you switch over to AF_INET, that's when the fun really starts.
In general, you can never assume that if something sends "hello world\n", that recv() will always receive "hello world\n" in a single message. Sometimes, you may get "hel" and then later "lo world\n". It is YOUR job to detect this, and build a semantically complete message before trying to process it further.
The first two points are addressed by
int n = recv(s2, str, sizeof(str)-1, 0);  // -1 allows for the \0
if(n <= 0) {
    if (n < 0) perror("recv");
        done = 1;
} else {
    // make it a string
    str[n] = '\0';
}



7. Parsing garbage.
                    char command[2222];


                    char * tmp;

                    i=0;

                    tmp = strtok (command," ");
                    arg_list[0] = tmp;


So, what's in command?
I thought the string was in str (or str2)

8. Bogus characters
> if(str2[0] >= '+' && str2[0] <= 'DEL')
What is 'DEL' supposed to be?
Was This Post Helpful? 1
  • +
  • -

#3 fredszky  Icon User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 7
  • Joined: 28-January 13

Re: execvp() and my program gets stuck, send() and it works.

Posted 07 March 2013 - 03:14 AM

Thank you for your reply.

Yes i know my coding skills are novice at best, i will look into each and every of your points.

Any idea why the program wont keep running after execvp()? With send() instead it keeps looping as it should.

execvp() used to work just fine and looping some days ago, i don't know why it suddenly doesn't. Maybe after i fiddled around with fork()..?
Was This Post Helpful? 0
  • +
  • -

#4 jjl  Icon User is offline

  • Engineer
  • member icon

Reputation: 1270
  • View blog
  • Posts: 4,997
  • Joined: 09-June 09

Re: execvp() and my program gets stuck, send() and it works.

Posted 07 March 2013 - 08:56 PM

execvp will replace the current process image with the program specified as the parameter. No instructions after an execvp call will be executed because they will no longer exists (that is that execvp was successful)

i.e.
int main() {
   execvp("program", NULL);
   fprintf(stderr, "ERROR: SHOULD NEVER GET HERE!");
   
   return 0; //this isn't even necessary
}


This post has been edited by jjl: 07 March 2013 - 09:00 PM

Was This Post Helpful? 0
  • +
  • -

Page 1 of 1