Krdytkyu

This is a simple program to run a command-line program with given parameters and verify that it writes the expected output to stdout or stderr. The specification file is passed as the first argument. It's formatted like this:

echo foo -> foo

man -> What manual page do you want?

with program and arguments to the left of -> and output on the right.

I don't often work so low-level, so I'm most interested in whether I'm managing resources properly.

#define _GNU_SOURCE



#include <errno.h>

#include <stdbool.h>

#include <stdio.h>

#include <stdlib.h>

#include <string.h>

#include <unistd.h>





#define MAX_ARGS 10

#define MAX_LINE 256





const char* parse_args(char **args, const char *line)

{

    size_t index = 0;

    size_t delimiter_index = 0;

    while (args[index] != NULL && index < MAX_ARGS - 1)

    {

        ++index;

        args[index] = strtok(NULL, " ");

        if (args[index] && strncmp(args[index], "->", 2) == 0)

        {

            delimiter_index = index;

            args[index+1] = strtok(NULL, "n");

            break;

        }

    }



    args[delimiter_index] = '';

    const char *expected_result = args[delimiter_index + 1];

    if (delimiter_index == 0 || expected_result == NULL)

    {

        fprintf(stderr, "Line <%s> is malformedn", line);

        return NULL;

    }



    if (strlen(expected_result) > MAX_LINE)

    {

        fprintf(stderr,

                "Invalid specification. Only results less than %d chars supported",

                MAX_LINE);

        return NULL;

    }



    return expected_result;

}



bool run_test(char *line)

{

    char *args[MAX_ARGS] = {};

    args[0] = strtok(line, " ");

    if (!args[0]) return true;



    const char* expected_result = parse_args(args, line);

    if (!expected_result)

    {

        return false;

    }



    int stdout_pipe[2];

    if (pipe(stdout_pipe) != 0)

    {

        fprintf(stderr, "Unable to open pipen");

        return false;

    }



    pid_t pid = fork();

    if (pid == -1)

    {

        fprintf(stderr, "Unable to fork processn");

        return false;

    }



    if (pid == 0)

    {

        // Redirect streams so caller can read stdout

        dup2(stdout_pipe[1], STDOUT_FILENO);

        dup2(stdout_pipe[1], STDERR_FILENO);

        close(stdout_pipe[0]);

        execvp(args[0], args);



        // exec* only ever returns if it fails

        fprintf(stderr, "Failed to execute test program: %sn",

                strerror(errno));

        return false;

    }



    close(stdout_pipe[1]);



    char actual_result[MAX_LINE] = {};

    ssize_t bytes_read = read(stdout_pipe[0], actual_result, MAX_LINE);

    if (bytes_read == -1)

    {

        fprintf(stderr, "Unable to read program outputn");

        return false;

    }



    // Strip newline

    if (actual_result[bytes_read - 1] == 'n')

    {

        actual_result[bytes_read - 1] = '';

    }



    if (strcmp(actual_result, expected_result) != 0)

    {

        fprintf(stderr, "Test Failed: ");

        for (int arg = 0; arg < MAX_ARGS; ++ arg)

        {

            if (!args[arg]) break;

            fprintf(stderr, "%s ", args[arg]);

        }

        fprintf(stderr, "n");

        fprintf(stderr, "Expected: %sn", expected_result);

        fprintf(stderr, "Actual: %sn", actual_result);

        return false;

    }



    return true;

}



int main(int argc, char *argv)

{

    if (argc != 2)

    {

        fprintf(stderr, "Usage: %s <test_specification>n", argv[0]);

        return 1;

    }



    const char *spec_pathname = argv[1];

    FILE* spec = fopen(spec_pathname, "r");

    if (!spec)

    {

        fprintf(stderr, "Cannot open <%s> for readingn", spec_pathname);

        return 1;

    }



    int return_code = 0;



    char *line = NULL;

    size_t line_length = 0;

    while (getline(&line, &line_length, spec) != -1)

    {

        if (!run_test(line))

        {

            return_code = 1;

        }

    }



    free(line);

    return return_code;

}

• 
  

  fprintf(stderr, "Unable to read program outputn"); loses important information: exactly why the program output couldn't be read. Prefer perror.

  
  
  

  Ditto for other fprintf(stderr, ....).

  
  



• The parent doesn't wait for children. You may produce plenty of zombies.




• The parent doesn't close(stdout_pipe[0]); when done with a child. You may run out of file descriptors.



• 
  

  The spec file format seems naive:

  
  
  
  
  
  • There is no way to deal with a multi-line output.
  
  
  • It mixes child's stdout and stderr. If the child produces both, their order is unpredictable.
  
  
  • It doesn't specify the child's return status.
  
  
  
  

while (args[index] != NULL && index < MAX_ARGS - 1)

I suggest that you refactor this loop a little bit; here's one option:

for (size_t index = 1; index < MAX_ARGS && args[index]; index++) { // ...

This:

args[delimiter_index] = '';

if it works without warning, that's only by accident. You're confusing pointer assignment with character assignment. It should actually be

args[delimiter_index] = NULL;

In all likelihood, both parse_args and run_test should be static because no one else is importing them.

For code like

if (pipe(stdout_pipe) != 0)

You should strongly consider calling perror to get a human-readable error string.

You can combine these fprintf calls and still have the strings on separate lines:

    fprintf(stderr, "n");

    fprintf(stderr, "Expected: %sn", expected_result);

    fprintf(stderr, "Actual: %sn", actual_result);

can be something like

    fprintf(stderr, "n"

                    "Expected: %sn"

                    "Actual: %sn", expected_result, actual_result);