I have a simple code that suppose to read length sized and fetch data from character device in Linux C.

This is my code and errno is set to 9. I made sure the file exists and it does. And able to read it with cat /dev/mychardev-0 but why bad file descriptor error at time of read. I am getting this line passed int fd=open("/dev/mychardev-0",O_RDONLY); if(fd<0)

#include <fcntl.h> 
#include <stdio.h> 
#include <string.h>
#include <unistd.h> 
#include <stdlib.h>
#include <errno.h>
int main()
{

    int fd=open("/dev/mychardev-0",O_RDONLY);
    if(fd<0)
    {   
        printf("fd %d\n",fd);
        exit(0);

    }

    int length=1024000;
    int x=0;
    char buffer[length-1];
    while(x<length)
    {
        int valread=read(fd,&buffer[x],length);
        if(valread==0)
        {
            printf("zero bytes read\n");
            
            break;
        }
        
        if(valread<0)
        {
            printf("read return negative %d %d\n",valread,errno);

            break;
        }
        x=x valread;
        
    }
    if(x>0)
    {
        printf("%x",buffer);
    }
    else
    {
        printf("ops no read\n");
    }
    return 0;
}

CodePudding user response：

char buffer[length-1];
int valread=read(fd,&buffer[x],length);

That is a very bad idea. You allocate enough space for length - 1 bytes then attempt to read length bytes into that space.

If the read gets the largest possible size, you will have a buffer overflow, well into undefined behaviour territory⁽¹⁾.

And, on top of that, you attempt to read length bytes even though you may have already read some in earlier iterations of the loop. You should be:

• Using the correct length for the buffer; and
• Adjusting the length down based on what you've already read.

And, though this may not be a problem, one meg is quite a lot of data to put on the stack (which is often limited in size by default). If that is the case, you may be better of using malloc to get a dynamic buffer, such as:

char *buffer = malloc(length);
// make sure buffer != NULL, then use it.
free(buffer);

So, something like (untested but you should hopefully get the idea):

int x = 0, length = 1024000; // with malloc if needed.
char buffer[length];
while (x < length) {
    int valread = read(fd, &buffer[x], length - x);
    if (valread == 0) {
        printf("zero bytes read\n");
        break;
    }
    else if (valread < 0) {
        printf("read return negative %d %d\n",valread,errno);
        break;
    }

    x  = valread;
}

⁽¹⁾ An example of what may happen is that overflowing the buffer will affect other variables on the stack in certain ways.

For example, it may corrupt x which would mean your next read could go to some arbitrary place in memory possibly corrupting other things as a result.

Or it could corrupt fd which would make the next read likely to fail with an invalid file descriptor.You can check the actual behaviour by simply printing out those two values before any use. Given the fact you mention "bad file descriptor error", that's possibly the most likely scenario here.

But, to be honest, the best solution is probably just to avoid undefined behaviour (though it may still be educational for you to find out the effects).