2

here's my whole code first :

 1. #include <stdio.h>
 2. #include <stdlib.h>
 3. #include <unistd.h>
 4. #include <sys/wait.h>
 5. #include <string.h>
 6. int main(int argc, char *argv[]) {
 7.     int p[2]; // p[0]: file descriptor for read end of pipe
 8.               // p[1]: file descriptor for write end of pipe
 9.     if (pipe(p) < 0) exit(1);
 10.    int rc1 = fork();
 11.    if (rc1 < 0){ fprintf(stderr, "fork error\n"); }
 12.    else if (rc1 == 0){ write(p[1], "1st child output",
 13.                              sizeof("1st child output")); }
 14.    else{
 15.        int rc2 = fork();
 16.        if (rc2 < 0){ fprintf(stderr, "fork error\n"); }
 17.        else if (rc2 == 0){
 18.            printf("2st child output\n");
 19.            char *_1st_child_out;
 20.            read(p[0], _1st_child_out, sizeof("1st child output"));
 21.            strcat(_1st_child_out, ", AFTER PIPE YA FOOL");
 22.            printf("%s\n", _1st_child_out);
 23.        }
 24.    }
 25. }

if i initialize 19:13:

char *_1st_child_out;

with a '\0' or NULL, the string stays empty and 22:13:

printf("%s\n", _1st_child_out);

prints nothing, so how do strcat() and read() work? should i not insert any null terminators before invoking them? what about garbage values?

  • 1
    `read(p[0], _1st_child_out, sizeof("1st child output"))` is reading into an uninitialized pointer, which is invalid. – Ry- Jul 29 '18 at 00:07
  • 1
    it reads input from the file descriptor provided in p[0], (which gets written in the pipe() call), and stores what has been read (i.e. what has been written in the write() above) to the _1st_child_out *char – Ahmad Rehan Jul 29 '18 at 00:11
  • 1
    I don't think the `pipe()` and `fork()` calls change anything -- could you reproduce the error with a smaller program? – SIGSTACKFAULT Jul 29 '18 at 00:29
  • 1
    @AhmedRehan: `_1st_child_out` is an uninitialized pointer. You can’t read anything into what it points to. – Ry- Jul 29 '18 at 00:30
  • `pipe()` doesn't do anything with `\0`. – user207421 Jul 29 '18 at 00:32
  • 1
    @Blacksilver as the code currently is, there isn't any errors, if i make the uninitialized char pointer "_1st_child_out" a NULL, nothing gets written or concatenated in it, and is i'm not fully grasping the effect of pipe, write & read i'm not sure which is causing the error and eitherway i'd like to know how both handle this situation – Ahmad Rehan Jul 29 '18 at 00:40
  • @Ry-♦ Oh ok, sorry i get what you meant now, i misunderstood "reading into", that means i should initialize my pointer first right, but initializing with NULL alone doesn't allow further writing or appending to it with write() and strcat() – Ahmad Rehan Jul 29 '18 at 00:44
  • 1
    @EJP thanks for pointing that out, sorry, i meant read() – Ahmad Rehan Jul 29 '18 at 00:47
  • Note that you should not create function, variable or macro names that start with an underscore, in general. [C11 §7.1.3 Reserved identifiers](https://port70.net/~nsz/c/c11/n1570.html#7.1.3) says (in part): — _All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use._ — _All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag name spaces._ See also [What does double underscore (`__const`) mean in C?](https://stackoverflow.com/a/1449301/15168) – Jonathan Leffler Jul 29 '18 at 03:06

3 Answers3

1

Your line 19 has a pointer that is not pointing to any allocated memory. Change it to a char array will fix the problem.

char _1st_child_out[100];

Also, for security, don't use strcat. All str function such as strcpy etc does not check destination boundary. They all have an n version. strcat should be replaced with strncat, which will take a third parameter to specify max length of cat so buffer overflow will not happen.

Jingshao Chen
  • 3,405
  • 2
  • 26
  • 34
1

READ(2) does not care about '\0', it does not care about the any value he reads. STRCAT(3) will read and write (operations, not functions) from the '\0' at the end of the given const char *src pointer.

From what I'm seeing, _1st_child_out is an uninitialized pointer. Semewhere in READ(2) there will be something as

dest[i] = array_bytes[i];

But here, your char *_1st_child_out is given a totaly "random" value and you will just write somewhere random in your memory, most of the time it will result in a Segmentation Fault, and your program will crash.

Then you need to take care about your use of sizeof operator here, you gives him "1st child output" which will be interpreted as a const char[] in C and will replace sizeof("1st child output") by the value 17 (16 characters and the '\0'). You need to specify a type in it, or a variable.

For arrays, like type array[x], sizeof(array) will be the same as sizeof(type) * x.

In order to fix your program, try creating a staticly allocated buffer like char _1st_child_out[x]; where 'x' is the size (in bytes) of the buffer. Or try using MALLOC(3). Then fix your 3rd parameter of READ(2).

When you'll use STRCAT(3), you need to know that if the size of the given destination buffer is not long enough to contain ", AFTER PIPE YA FOOL" your program have a huge chance of crashing.

1

There are few bugs in your code, here are the my observation.

Case 1 :- In your code you are calling fork() two times, pipe write end p[1] contains some data 1st child output in first fork() rc1 process, but your code tried to read form p[0] in second rc2 process.

you should check read() return value whether it's success or not or may be it's reading from wrong/uninitialized file descriptors. This

    char *_1st_child_out = NULL;
    /* for process rc2, p[0] contains nothing, so what read() will read from p[0] ?? */
    int ret = read(p[0], _1st_child_out, sizeof("1st child output"));
    if(ret == -1) {
          perror("read");
          /* error handling */
   }

Since data written into p[1] in rc1 process, not in rc2 process, but here when you tries to read from p[0] it gives you

read: Bad address

Case 2 :- To overcome above problem one way is

int main(int argc, char *argv[]) {
        int p[2]; // p[0]: file descriptor for read end of pipe
        // p[1]: file descriptor for write end of pipe
        if (pipe(p) < 0) exit(1);
        int rc1 = fork();
        if (rc1 < 0){ fprintf(stderr, "fork error\n"); }
        else if (rc1 == 0){
                write(p[1], "1st child output",sizeof("1st child output"));
        }
        else{
                char *_1st_child_out = NULL;

                /* read() will read from p[0] and store into _1st_child_out but _1st_child_out not holding any valid memory ? So it causes Undefined behavior */
                int ret = read(p[0], _1st_child_out, sizeof("1st child output"));
                if(ret == -1) {
                perror("read");
                /* error handling */
                }
                strcat(_1st_child_out, ", AFTER PIPE YA FOOL");
                printf("%s\n", _1st_child_out);
        }
        return 0;
}

Here _1st_child_out is a pointer and pointers should have valid memory location. you can initialize with NULL which is (void*)0, that's valid but not with \0 as it's just a single character.

But when you initialize _1st_child_out with NULL and read the data from p[0] & store into _1st_child_out, what it will store into it ? It causes segmentation fault and it's also undefined behavior.

So it's better to allocate memory dynamically for _1st_child_out and then call read() or create the stack allocated array like

char _1st_child_out[10];

Here is the sample working code

int main(int argc, char *argv[]) {
        int p[2]; // p[0]: file descriptor for read end of pipe
        // p[1]: file descriptor for write end of pipe
        if (pipe(p) < 0) exit(1);
        int rc1 = fork();
        if (rc1 < 0){ fprintf(stderr, "fork error\n"); }
        else if (rc1 == 0){
                write(p[1], "1st child output",sizeof("1st child output"));
        }
        else{

                char *_1st_child_out = malloc(BYTE); /* define BYTE value as how much memory needed, and free the dynamically allocated memory once job is done */
                int ret = read(p[0], _1st_child_out, sizeof("1st child output"));
                if(ret == -1) {
                        perror("read");
                        /* error handling */
                }
                /* make sure _1st_child_out has enough memory space to concatenate */
                strcat(_1st_child_out, ", AFTER PIPE YA FOOL");
                printf("%s\n", _1st_child_out);
        }
        return 0;
}

Note : Use strncat() instead of strcat(), reason you can find from manual page of strcat() https://linux.die.net/man/3/strcat It says

The strcat() function appends the src string to the dest string, overwriting the terminating null byte ('\0') at the end of dest, and then adds a terminating null byte. The strings may not overlap, and the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable; buffer over‐ runs are a favorite avenue for attacking secure programs.

   The strncat() function is similar, except that

   *  it will use at most n bytes from src; and

   *  src does not need to be null-terminated if it contains n or more bytes.

   As with `strcat()`, the resulting string in dest is always null-terminated.
Achal
  • 11,821
  • 2
  • 15
  • 37
  • 1
    what is the optimal way to create a pipe between two children of the same parent? (this was specifically asked in a book assignment) And why might the read() be reading from a wrong/uninitialized file descriptor? The first couple of google results never go in depth about these these functions, is there a specific documentation that goes over them more thoroughly? – Ahmad Rehan Jul 30 '18 at 01:18