0

im trying to run this on Linux, this program supposed to pass arrays of numbers with pipes, to the children, and each children calculate the gcd of the pairs. But I get "Segmentation fault(core dumped)" ERROR. I've checked the child's process, and right after the read() i tried to print string just for check and it doesn't work. The weird thing that the read not returns -1, which means it worked. is it possible to write char **arr; into a pipe? or is it to big for a pipe and this is why it crashes. Thank you for any help.

BTW, the ./v2_child1 is fine, the problem comes before the execvp()

#include <stdarg.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX_LINE_LENGTH 100
#define FILE_NAME "numbers.txt"

char** readFromTextFile(char *fileName) {

    FILE *f = fopen(fileName, "r");
    if (f == NULL) {
        printf("Error while opening file!");
        return NULL;
    }
    char *line = (char*) malloc(MAX_LINE_LENGTH * sizeof(char));
    char **pairs = (char**) malloc(50 * sizeof(char*));
    int counter = 0;

    while (!feof(f)) {
        char *num1 = (char*) malloc(sizeof(char) * 2);
        char *num2 = (char*) malloc(sizeof(char) * 2);
        fgets(line, MAX_LINE_LENGTH, f);
        sscanf(line, "%s   %s", num1, num2);
        pairs[counter] = num1;
        pairs[counter + 1] = num2;
        counter += 2;
    }
    pairs[counter] = NULL;
    fclose(f);
    return pairs;
}

int numOfPairs(char **arr) {
    int count = 0;
    while (arr[count] != NULL) {
        count += 1;
    }
    if ((count % 2) != 0) {
        printf("odd amount off numbers");
        return -1;
    } else {
        return count / 2;
    }
}

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

    //read the pairs of numbers into array
    char **numbers = readFromTextFile(FILE_NAME);
    //returns the num of pairs to check
    int num_pairs = numOfPairs(numbers);
    //initialize the pipes
    int write_pipe[2], read_pipe[2];

    if (pipe(write_pipe) == -1 || pipe(read_pipe) == -1) {
        fprintf(stderr, "Pipe operation failed!");
        exit(0);
    }

    //child --> parent
    int PARENT_READ = read_pipe[0]; // IN
    int CHILD_WRITE = read_pipe[1]; // OUT

    //parent --> child
    int CHILD_READ = write_pipe[0];
    int PARENT_WRITE = write_pipe[1];

    pid_t status = fork(); // create child number 1

    if (status < 0) { // error ocurred
        fprintf(stderr, "Error with fork");
        exit(0);

    } else if (status > 0) { // parent go here 

        char **to_child1 = (char**) malloc(sizeof(char*) * (num_pairs / 2) * 2);
        for (int i = 0; i < num_pairs / 2; ++i) {
            to_child1[2 * i] = numbers[2 * i];
            to_child1[2 * i + 1] = numbers[2 * i + 1];
        }

        if (close(CHILD_READ) == -1)
            perror("problem while close CHILD_READ");
        if (write(PARENT_WRITE, to_child1, sizeof(char*) * (num_pairs / 2) * 2)
                == -1)
            perror("problem while write to PARENT_WRITE");

        if (close(PARENT_WRITE))
            perror("problem while close PARENT_WRITE");

        printf("wrote from parent to pipe\n\n");


    } else { // child process
        char **first_half = (char**) malloc(
                sizeof(char*) * (num_pairs / 2) * 2);
        printf("Hello form son 1\n");
        if (close(PARENT_WRITE) == -1)
            perror("Error while close");
        read(CHILD_READ, first_half, sizeof(char*) * (num_pairs / 2) * 2);

        printf("child got here"); // not printing this*

        if (close(PARENT_READ) == -1) //read is unused
            perror("Error while close");
        if (dup2(CHILD_WRITE, STDOUT_FILENO) == -1) { //redirecting Stdout to pipe.
            perror("dup2 error");
        }

        char *args[num_pairs / 2 + 1];
        args[0] = "./v2_child1";
        for (int i = 1; i < num_pairs / 2 + 1; ++i) {
            args[i] = first_half[i];
        }
        execvp(args[0], args);

    }
    wait(NULL);
    char **gcds = (char**) malloc(sizeof(char*) * (num_pairs / 2));
    close(CHILD_WRITE);
    read(PARENT_READ, gcds, sizeof(int));
    for (int i = 0; i < num_pairs / 2; ++i) {
        printf("The gcd of %d and %d is: %d - calculated from child 1\n",
                atoi(numbers[i * 2]), atoi(numbers[i * 2 + 1]), atoi(gcds[i]));
    }

    /// another child to be created
}

  • 1
    [dont cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar Apr 23 '20 at 21:43
  • 2
    [Why `while(!feof(file))` is always wrong](https://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) – Barmar Apr 23 '20 at 21:43
  • `malloc(sizeof(char)*2)` only allocates enough space for 1-character strings, since you need 1 byte for the null terminator. Are all the numbers in the file just one digit? – Barmar Apr 23 '20 at 21:48
  • This ranks as pretty much the least efficient way ever to allocate a pair of single char with NUL term. Hoping the small object allocater can hide all that overhead. – Michael Dorgan Apr 23 '20 at 21:51

1 Answers1

1

to_child1 is an array of pointers. Pointer values are only meaningful within the process that created them. Writing a pointer into the pipe does not copy the data that it points to. So when the child process reads the pointers, it doesn't have the strings they point to.

Since all the strings are just 1 character, there's no need to use an array of pointers, just make an array of char, and write that to the pipe.

Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Thanks. actually the strings are not all one digit numbers. it can be 2 digit as well thats why the array of pionters. – Evyatar Menczer Apr 25 '20 at 08:18
  • But you're allocating only 2 bytes for each pointer. Since a string has a null terminator byte, that's only room for one digit. – Barmar Apr 25 '20 at 12:27