How to read a line from a read-only FIFO in C?

Question

I've got a problem reading a couple of lines from a read-only FIFO. In particular, I have to read two lines — a number n, followed by a \n and a string str — and my C program should write str in a write-only FIFO for n times. This is my attempt.

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <ctype.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>

char *readline(int fd);

int main(int  argc, char** argv) {
    int in = open(argv[1], O_RDONLY);
    mkfifo(argv[2], 0666);
    int out = open(argv[2] ,O_WRONLY);
    char *line = (char *) malloc(50);
    int n;

    while (1) {
        sscanf(readline(in), "%d", &n);
        strcpy(line, readline(in));

        int i;
        for (i = 0; i < n; i++) {
            write(out, line, strlen(line));
            write(out, "\n", 1);
        }
    }

    close(in);
    close(out);
    return 0;
}

char *readline(int fd) {
    char *c = (char *) malloc(1);
    char line[50];

    while (read(fd, c, 1) != 0) {
        if (strcmp(c, "\n") == 0) { 
            break;
        }
        strcat(line, c);
    }
    return line;
}

The code is working properly, but it puts a random number of newlines after the last string repetition. Also, this number changes at each execution.

Could someone please give me any help?

`char *c = (char *) malloc(1);` a 1 char buffer will not have enough place for a character plus a null terminator; the `strcat(line, c);` will expect its arguments both to be nul-terminated. — wildplasser, Nov 09 '13 at 17:41

alk · Answer 1 · 2013-11-09T20:09:44.363

Besides the facts that reading character wise and and comparing two characters using "string" comparsion both is far from being efficient, readline() returns a pointer to memory being declared local to readline(), that is line[50] The memory gets deallocated as soon as readline() returns, so accessing it afterwards invokes undefine behaviour.

One possibility to fix this is to declare the buffer to read the line into outside readline() and pass a reference to it down like so:

char * readline(int fd, char * line, size_t size) 
{
  if ((NULL != line) && (0 < size))
  {
    char c = 0;
    size_t i = 0;
    while (read(fd, &c, 1) >0) 
    {
      if ('\n' == c) or (size < i) { 
        break;
      }
      line[i] = c;
      ++i;
    }
    line [i] = 0;
  }

  return line;
}

And then call it like this:

char * readline(int fd, char * line, size_t size);

int main(void)
{
  ...
  char line[50] = "";
  ...
  ... readline(in, line, sizeof(line) - 1) ...

score 0 · Answer 2 · edited May 23 '17 at 10:31

0

I have not tried running your code, but in your readline function you have not terminated the line with null ('\0') character. once you hit '\n' character you just breaking the while loop and returning the string line. Try adding '\0' character before returning from the function readline.

Click here for more info.

edited May 23 '17 at 10:31

Community

1
1

answered Nov 09 '13 at 17:43

Basavaraju B V

174
1
8

score -1 · Accepted Answer · edited May 23 '17 at 10:31

Your code did not work on my machine, and I'd say you're lucky to get any meaningful results at all.

Here are some problems to consider:

readline returns a locally defined static char buffer (line), which will be destroyed when the function ends and the memory it once occupied will be free to be overwritten by other operations.
If line was not set to null bytes on allocation, strcat would treat its garbage values as characters, and could possibly try to write after its end.
You allocate a 1-byte buffer (c), I suspect, just because you need a char* in read. This is unnecessary (see the code below). What's worse, you do not deallocate it before readline exits, and so it leaks memory.
The while(1) loop would re-read the file and re-print it to the output fifo until the end of time.
You're using some "heavy artillery" - namely, strcat and memory allocation - where there are simpler approaches.
Last, some C standard versions may require that you declare all your variables before using them. See this question.

And here's how I modified your code. Note that, if the second line is longer than 50 characters, this code may also not behave well. There are techniques around the buffer limit, but I don't use any in this example:

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
#include <ctype.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <string.h>

char *readline(int fd, char * buffer);

int main(int  argc, char** argv) {
    int in = open(argv[1], O_RDONLY);
    int out;
    int n;
    int i;
    char line[50];

    memset(line, 0, 50);
    mkfifo(argv[2], 0666);
    out = open(argv[2] ,O_WRONLY);

    sscanf(readline(in, line), "%d", &n);
    strcpy(line, readline(in, line));

    for (i = 0; i < n; i++) {
        write(out, line, strlen(line));
        write(out, "\n", 1);
    }

    close(in);
    close(out);
    return 0;
}

char *readline(int fd, char * buffer) {
    char c;
    int counter = 0;
    while (read(fd, &c, 1) != 0) {
        if (c == '\n') {
            break;
        }
        buffer[counter++] = c;
    }
    return buffer;
}

This works on my box as you described. Compiled with GCC 4.8.2 .

Thank you, this works fine for me, too. I just only got a last question: how can the write-only FIFO be opened _before_ its creation with `mkfifo`? — Dree, Nov 09 '13 at 19:02
Well, I suppose that the `out` FIFO doesn't exist after its creation with `mkfifo(argv[2], 0666)`, so I wonder how I can open it earlier (i. e. the `open` call comes first). Sorry, this might seem a stupid question for you, but I'm pretty new to FIFOs and I had to make a big effort to understand them. — Dree, Nov 09 '13 at 19:51
Oh, I had an error trying to access a named pipe before creating it. Only reason it worked was a named pipe left over from the previous run - edited. That was what you were asking, right? — mcmlxxxvi, Nov 09 '13 at 20:18

How to read a line from a read-only FIFO in C?

3 Answers3