[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gpsd-commit-watch
Subject:    [Gpsd-commit-watch] r3160 - trunk
From:       ckuethe () berlios ! de (Chris Kuethe at BerliOS)
Date:       2005-08-19 18:05:49
Message-ID: 200508191805.j7JI5nsG029527 () sheep ! berlios ! de
[Download RAW message or body]

Author: ckuethe
Date: 2005-08-19 20:05:48 +0200 (Fri, 19 Aug 2005)
New Revision: 3160

Modified:
   trunk/packet.c
Log:
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.


Modified: trunk/packet.c
===================================================================
--- trunk/packet.c	2005-08-19 16:23:53 UTC (rev 3159)
+++ trunk/packet.c	2005-08-19 18:05:48 UTC (rev 3160)
@@ -791,12 +791,20 @@
 {
     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);
 }


[prev in list] [next in list] [prev in thread] [next in thread]