On 11/27/2009 06:20 PM, cmccabe@xxxxxxxxxxxxxx wrote:
--- a/lib/common.c
+++ b/lib/common.c
@@ -56,6 +56,37 @@ const char *cld_errstr(enum cle_err_codes ecode)
return cld_errlist[ecode];
}
+/** Read from a file descriptor, resuming after interruptions.
+ *
+ * @param fd The file descriptor
+ * @param buf Output buffer
+ * @param len How many bytes to read
+ *
+ * @return Negative error code on error, or the number of bytes
+ * that were read. This will only be less than the
+ * requested length if the file is too short.
+ */
+int safe_read(int fd, void *buf, int len)
+{
+ int rem = len;
+ while (rem) {
+ int res = read(fd, buf, rem);
+ if (res == 0) {
+ return len - rem;
+ }
+ else if (res< 0) {
+ int err = errno;
+ if (err != EINTR)
+ return -err;
+ }
+ else {
+ rem -= res;
+ buf += res;
+ }
+ }
+ return len;
+}
+
/*
* Read a port number from a port file, return the value or negative error.
*/
@@ -69,13 +100,13 @@ int cld_readport(const char *fname)
if ((fd = open(fname, O_RDONLY)) == -1)
return -errno;
- rc = read(fd, buf, LEN);
+ memset(buf, 0, sizeof(buf));
+ rc = safe_read(fd, buf, LEN);
close(fd);
if (rc< 0)
- return -errno;
+ return rc;
if (rc == 0)
return -EPIPE;
- buf[rc] = 0;
Well, while technically correct, nobody really bothers with EINTR or
loops when reading from a file, because all Unix/Linux/BSD kernels have
historically given you exactly the amount you needed. Any stray EINTR
is often something serious that indicates the program should abort --
Ctrl-C at terminal by an impatient user, or a kill signal from an admin.
Also, for reading whole files into memory, GLib's g_file_get_contents()
already exists for that. It does full error checking.
I do wonder what cygwin does, and if a read(2) loop matters on
Windows...? Eventually I would like to see these programs portable
enough to run under mingw or cygwin.
diff --git a/test/it-works.c b/test/it-works.c
index bd2f965..0e932e7 100644
--- a/test/it-works.c
+++ b/test/it-works.c
@@ -107,6 +107,7 @@ static int init(void)
int main (int argc, char *argv[])
{
+ g_thread_init(NULL);
cldc_init();
event_init();
if (init())
diff --git a/test/load-file-event.c b/test/load-file-event.c
index 1620508..23ee4e0 100644
--- a/test/load-file-event.c
+++ b/test/load-file-event.c
@@ -243,6 +243,7 @@ int main(int argc, char *argv[])
}
#endif
+ g_thread_init(NULL);
cldc_init();
event_init();
#if 0
diff --git a/test/lock-file-event.c b/test/lock-file-event.c
index c69da66..6fc4f1a 100644
--- a/test/lock-file-event.c
+++ b/test/lock-file-event.c
@@ -247,6 +247,7 @@ static int init(void)
int main (int argc, char *argv[])
{
+ g_thread_init(NULL);
cldc_init();
event_init();
if (init())
diff --git a/test/save-file-event.c b/test/save-file-event.c
index 1b3418a..23f1781 100644
--- a/test/save-file-event.c
+++ b/test/save-file-event.c
@@ -249,6 +249,7 @@ int main(int argc, char *argv[])
}
#endif
+ g_thread_init(NULL);
cldc_init();
event_init();
This is OK, but should be in a separate patch.
Similar to Linux kernel changes, logically separate changes should be in
distinct, separate patches.
That allows bisection ('git bisect') to better isolate problematic
changes, and helps reviewers. It also means I can immediately apply one
patch, while requesting revisions of another patch. Introduces fewer
"stalls" in the programming pipeline.
Jeff
--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
