Changes to read() error handling.

General feeling seems to be:
a) that checking for (newdata = read()) == -1 is good,
b) that it is appropriate to retry in case of EINTR or EAGAIN
c) a short delay in case of EINTR or EAGAIN is polite but not strictly needed.

I put the delay in because this may spin and I really hate busy waiting.
I may put in a retry counter: after some small number of retries that fail,
just give up and return 0 as if there was no data to be read. We shouldn't
have even reached this function unless select() said there was data to read
so it seems appropriate to get the data (with retries if necessary) unless a
truly fatal error occurred ((errno != EAGAIN) && (errno != EINTR))...

As we've just tagged a release, I feel better about committing this so this
gets a bit more testing.

1 files changed, 11 insertions, 3 deletions

diff --git a/packet.c b/packet.c
index cc24739c..c54091d8 100644
--- a/packet.c
+++ b/packet.c
@@ -791,12 +791,20 @@ ssize_t packet_get(struct gps_device_t *session)
 {
     ssize_t newdata;
     /*@ -modobserver @*/
+loop:
     newdata = read(session->gpsdata.gps_fd, session->inbuffer+session->inbuflen,
 			sizeof(session->inbuffer)-(session->inbuflen));
     /*@ +modobserver @*/
-    if (newdata < 0 && errno != EAGAIN)
-	return BAD_PACKET;
-    else if (newdata == 0 || (newdata < 0 && errno == EAGAIN))
+    if (newdata == -1){ /*newdata == -1 && errno = (EAGAIN || EINTR) */
+        if ((errno == EAGAIN) || (errno == EINTR)) {
+	    usleep(10000);
+	    goto loop;
+        } else {
+	    return BAD_PACKET;
+        }
+    }
+
+    if (newdata == 0)
 	return 0;
     return packet_parse(session, (size_t)newdata);
 }