1

The code below returns EFAULT (errno == 14). I would appreciate help figuring out why.

I've also tried to implement the code using select() but still got the same error code.

I've got very similar code running on Python with no issues.

#include <stdio.h> 
#include <string.h> 
#include <fcntl.h> 
#include <sys/stat.h> 
#include <unistd.h> 
#include <errno.h> 



int read_fail1(int fd)
{
    int n;
    char buf[500];

    for (;;)
    {
        buf[strlen(buf)-1] = 0;
        n = read(fd, buf, strlen(buf)-1);
        
        if (n == -1)
        {
            if (errno == EFAULT)
            {
                fprintf(stderr, "EFAULT");
                return 42;
            }
        }
        else if (n > 0)
        {
            fprintf(stderr, "%s", buf);
        }

    }
}



int main(int argc, const char *argv[])
{
    const char *myfifo = "pipeMUD";
  
    mkfifo(myfifo, 0666); 
  
    int fd = open(myfifo, O_RDWR | O_NONBLOCK);

    if (fd <= 0)
        return 42;

    read_fail1(fd);

    return 0;
}

POST ANSWER EDIT:

As mentioned in the post linked below, if an invalid address is passed to the kernel, it throws the EFAULT. I guess that on Linux, based on the above code, passing a 0 length count parameter to read() will also cause EFAULT to be retured.

unix socket error 14: EFAULT (bad address)

Michael Seifert
  • 162
  • 11
  • 2
    `buf[strlen(buf)-1] = 0;` That's undefined behaviour as `buf` does not contain a valid string yet when it runs. – kaylum Jan 10 '21 at 19:53
  • can't believe I didn't see that. Sorry. Embarrassing – Michael Seifert Jan 10 '21 at 21:35
  • the posted code does not cleanly compile! For instance, `int main(int argc, const char *argv[])` results in two warning messages about unused parameters. (suggest using: `int main( void )`. When compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pendantic -std=gnu11` ) Note: other compilers use different options to produce the same results. – user3629249 Jan 13 '21 at 07:35
  • regarding: `n = read(fd, buf, strlen(buf)-1);` the function: `read()` returns a `ssize_t` not a `int` – user3629249 Jan 13 '21 at 07:41
  • regarding: `int fd = open(myfifo, O_RDWR | O_NONBLOCK);` since the posted code is only reading from the FIFO, should only open for reading, not writing – user3629249 Jan 13 '21 at 08:06
  • since the read is to a 'non-blocking' read(), suggest checking for `EAGAIN` when the returned byte count is 0 – user3629249 Jan 13 '21 at 08:10
  • regarding `buf[strlen(buf)-1] = 0; n = read(fd, buf, strlen(buf)-1);` It would be MUCH better to use: `ssize_t n = read( fd, buf, sizeof( buf )-1 ); If ( n > 0 ) { buf[ n ] = '\0'; } else if( n == 0 ) { then no bytes available } else if( n<0 ) { /* handle error */ }` – user3629249 Jan 13 '21 at 08:11

1 Answers1

1

This line:

buf[strlen(buf)-1] = 0;

buf if a local variable, and thus is not initialized in C. strlen looks for '\0' (null character) value, and thus will give unpredictable result on uninitialized array.

But, as long as you declare buf statically as you do, you can use sizeof instead. Though it would be a better practice to use a macro instead:

#define READ_BUFFER_SIZE 500

char buf[READ_BUFFER_SIZE];
n = read(fd, buf, READ_BUFFER_SIZE - 1);
Lev M.
  • 6,088
  • 1
  • 10
  • 23