Argthtjtr

I've been working on the following code for quite some time, but can't really figure it out.
The task is to read a terminal command and to run it every x seconds; if the command hasn't finished within the waiting time, we want to kill the process and afterwards run the command again.
Any help would be really appreciated.
I'm pretty sure I'm not using waitpid() correctly; how would I go about using waitpid to achieve the goal?

Additionally, how would I go about detecting an error within a child process? The plan is to kill the parent process if an error occurred in a child process.

#include <stdio.h>

#include <stdlib.h>

#include <unistd.h>

#include <sys/types.h>

#include <string.h>

#include <signal.h>

#include <sys/wait.h>



/*

 * 

 */

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

    int waitingTime;

    if (argc < 3) {

        printf("Invalid number of arguments provided. Please specify a command and exactly one parameter.");

        return (EXIT_FAILURE);

    } 

    // -n parameter specified? If so, set the waiting time.

    if (argc == 5 && strcmp(argv[3], "-n") == 0) {

        waitingTime = atoi(argv[4]);

    } else {

        waitingTime = 5; // Default waiting time.

    }



    char* cmd = (char*)malloc(sizeof(argv[1]));

    cmd = argv[1];

    char* param = (char*)malloc(sizeof(argv[2]));

    param = argv[2];



    // Print the read command and its param

    printf("Command: %s, Parameter: %s, Interval: %dnn", cmd, param, waitingTime);    



    pid_t pid;



    for (;;) { 

        // Declared here for scope

        int secsWaited;

        secsWaited = 0;

        pid = fork();



        if (pid == 0) {

            pid = getpid();

            printf("==============n");

            execlp(cmd, cmd, param, "/", (char *)NULL);

            printf("Excec failed; killing the proccess.");

            kill(pid, SIGKILL);

        } else if (pid > 0) {

            int status, code;

            for (;;) {

                code = waitpid(pid, &status, WNOHANG);

            if (code == 0 && secsWaited >= waitingTime) {

                kill(pid, SIGKILL);

            printf("Child stopped");

            break;

        } else if (code == 0 && secsWaited < waitingTime) {

            secsWaited++;   

                    sleep(1);

                } else {

                    break;

                }

            }



            /*if (!WIFEXITED(status)) {

                printf("Time exceeding, stopping child.");

        // Get parent process id and kill it.

        kill(getpid(), SIGKILL);

        }*/







            // Sleep for the specified time

            sleep(waitingTime - secsWaited);



        } else {

            return (EXIT_FAILURE);

        }



    }      



    free(cmd);

    free(param);       

    return (EXIT_SUCCESS);

}

Your logic was a bit too complicated (e.g. too many different sleep calls and if/else ladder logic).

Also, no need to malloc the argv strings--they can be used directly.

I've simplified it and restructured a bit to get it to work [please pardon the gratuitous style cleanup]:

#include <stdio.h>

#include <stdlib.h>

#include <unistd.h>

#include <sys/types.h>

#include <string.h>

#include <signal.h>

#include <sys/wait.h>



/*

 *

 */

int

main(int argc, char **argv)

{

    int waitingTime;



    if (argc < 3) {

        printf("Invalid number of arguments provided. Please specify a command and exactly one parameter.");

        return (EXIT_FAILURE);

    }

    // -n parameter specified? If so, set the waiting time.

    if (argc == 5 && strcmp(argv[3], "-n") == 0) {

        waitingTime = atoi(argv[4]);

    }

    else {

        waitingTime = 5;                // Default waiting time.

    }



    char *cmd = argv[1];

    char *param = argv[2];



    // Print the read command and its param

    printf("Command: %s, Parameter: %s, Interval: %dnn",

        cmd, param, waitingTime);



    pid_t pid;

    int code = -1;

    int status;

    int killflg = 1;



    for (;;) {

        // Declared here for scope

        int secsWaited;



        secsWaited = 0;

        pid = fork();



        // stop on fork failure

        if (pid < 0) {

            killflg = 1;

            break;

        }



        // child process

        if (pid == 0) {

            pid = getpid();

            printf("==============n");

#if 0

            execlp(cmd, cmd, param, "/", (char *) NULL);

#else

            execlp(cmd, cmd, param, (char *) NULL);

#endif

            printf("Excec failed; killing the proccess.");



// NOTE/BUG: this is the child so pid is zero, so killing it is wrong

#if 0

            kill(pid, SIGKILL);

#else

            exit(1);

#endif

        }



        killflg = 0;

        for (;;) {

            code = waitpid(pid, &status, WNOHANG);

            if (code > 0)

                break;



            if (killflg)

                continue;



            secsWaited++;

            sleep(1);



            if (secsWaited >= waitingTime) {

                printf("timeoutn");

                kill(pid, SIGKILL);

                killflg = 1;

            }

        }



        if (! killflg)

            break;

    }



#if 0

    free(cmd);

    free(param);

#endif



    if (killflg)

        code = EXIT_FAILURE;

    else

        code = EXIT_SUCCESS;



    return code;

}

UPDATE:

Right now, the program will stop after one iteration; if I remove the breakpoint at if (! killflg), it will work as expected. Am I missing something or is this just a misunderstanding?

You are correct--my bad. I had missed the following in your question:

The task is to read a terminal command and to run it every x seconds;

Change the break into sleep(waitingTime - secsWaited).

But, a more robust way to keep track of elapsed time may be via two calls to time(2):

After the killflg = 0, do: time_t todbeg = time(NULL); time_t todelap;. Then, you can get elapsed time [anywhere] with: todelap = time(NULL) - todbeg; [here, todelap is similar to secsWaited]. This may be better than incrementing secsWaited.

time only has seconds resolution. For more precision control, consider using clock_gettime [has nanosecond resolution].

Here's a function that I use a lot for elapsed time [in fractional seconds]:

#include <stdio.h>

#include <stdlib.h>

#include <unistd.h>

#include <sys/types.h>

#include <string.h>

#include <signal.h>

#include <sys/wait.h>

#include <time.h>



double

tvgetf(void)

{

    struct timespec ts;

    double sec;



    clock_gettime(CLOCK_REALTIME,&ts);



    sec = ts.tv_nsec;

    sec /= 1e9;

    sec += ts.tv_sec;



    return sec;

}



/*

 *

 */

int

main(int argc, char **argv)

{

    int waitingTime;



    if (argc < 3) {

        printf("Invalid number of arguments provided. Please specify a command and exactly one parameter.");

        return (EXIT_FAILURE);

    }

    // -n parameter specified? If so, set the waiting time.

    if (argc == 5 && strcmp(argv[3], "-n") == 0) {

        waitingTime = atoi(argv[4]);

    }

    else {

        waitingTime = 5;                // Default waiting time.

    }



    char *cmd = argv[1];

    char *param = argv[2];



    // Print the read command and its param

    printf("Command: %s, Parameter: %s, Interval: %dnn", cmd, param, waitingTime);



    pid_t pid;

    int code = -1;

    int status;

    int killflg = 1;

    double todzero = tvgetf();



    for (;;) {

        // Declared here for scope

        double todbeg = tvgetf();

        double todelap;



        pid = fork();



        // stop on fork failure

        if (pid < 0) {

            killflg = 1;

            break;

        }



        // child process

        if (pid == 0) {

            pid = getpid();

            printf("============== (%.9f)n",tvgetf() - todzero);

            execlp(cmd, cmd, param, (char *) NULL);

            printf("Excec failed; killing the proccess.");

            exit(1);

        }



        killflg = 0;

        for (;;) {

            code = waitpid(pid, &status, WNOHANG);

            if (code > 0)

                break;



            if (killflg)

                continue;



            usleep(1000);



            todelap = tvgetf() - todbeg;

            if (todelap >= waitingTime) {

                printf("timeoutn");

                kill(pid, SIGKILL);

                killflg = 1;

            }

        }



        // do _not_ wait -- we already timed out

        if (killflg)

            continue;



        // get final elapsed time for this round and the amount of time

        // remaining until the next interval

        todelap = tvgetf() - todbeg;

        useconds_t time_to_wait = ((double) waitingTime - todelap) * 1e6;



        // wait until the next time period

        if (time_to_wait > 0)

            usleep(time_to_wait);

    }



    if (killflg)

        code = EXIT_FAILURE;

    else

        code = EXIT_SUCCESS;



    return code;

}

Side note: I used usleep here, but, although slightly more complex, it's considered better to use nanosleep