[BRLTTY] screen driver in brlapi

[Dave Mielke]

[quoted lines by Tage Johansson on 2018/11/07 at 06:33 +0100]

>I've now rewritten my code, but it still returns instantly.
...
>    if (vcsa == -1) {
>        printf("Error: %s\n", strerror(errno));
>        exit(EXIT_FAILURE);
>    }

You should consider prefixing each error message with the name of the operation
that failed. For example, instead of just "Error", I'd recommmend using
prefixes lke "open error", "read error", "poll error", etc.

>    struct pollfd fd = {
>        .fd = vcsa,
>        .events = POLLIN
>    };

I recommend moving this declaration inside the loop so that it'll be properly
reinitialized each time before it's used.

>    for(int i = 0; i < 30; i++) {
>        if (read(vcsa, header, sizeof(header)) == -1) {
>            printf("Error: %s\n", strerror(errno));
>            exit(EXIT_FAILURE);
>        }
>        if (lseek(vcsa, 0, SEEK_SET) == -1) {
>            printf("Error: %s\n", strerror(errno));
>            exit(EXIT_FAILURE);
>        }

My recommendation is to do the seek immediately before the read. This avoids
the poissibility that some new code might change the read position before the
read is done. It's easy, when code is still simple, to believe that one knows
what the code is doing, but, as code becomes more complex, this becomes more of
a delusion than a truism. My opinion is that it's always best to code safely so
that new code won't inadvertently introduce unwanted surprises.

You may remember that my sample code used pread() instead of read(). That's
because pread() takes a fourth parameter - the offset - which means that the
seek isn't even necessary.

>        if(poll(&fd, 1, -1) == -1) {
>            printf("Error: %s\n", strerror(errno));
>            exit(EXIT_FAILURE);
>        }

Strictly speaking, even though you're specifying a timeout of -1 (infinite), I
recommend testing for a return value of 0 (timed out) as well. This is, again,
me "lecturing" on my version of safe programming. My reason is that, some time,
the timeout might be changed to a non-infinite value, at wh9ch point the cocde,
as written, won't handle it. It's usually safest to handle all the possible
return values at the time the initial code is still being written, i.e. while
the details are still on your mind.

>I ran the program as root. The output looks like this:
>9 header: 37, 100, 0, 35
>1 header: 37, 100, 0, 35
...
>So the first call to poll indicates some error because the "POLLERR" bit is
>set. Subsiquent calls are fine because they only sets the POLLIN bit.

You need to monitor vcsa with POLLPRI (not POLLIN).

-- 
I believe the Bible to be the very Word of God: http://Mielke.cc/bible/
Dave Mielke            | 2213 Fox Crescent | WebHome: http://Mielke.cc/
EMail: Dave at Mielke.cc  | Ottawa, Ontario   | Twitter: @Dave_Mielke
Phone: +1 613 726 0014 | Canada  K2A 1H7   |