This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: BUG: alternatives


It would help if I actually attached the patch.

Charles Wilson wrote:
Christopher Faylor wrote:

A signal shouldn't cause a truncated read when retrieving data from
disk on cygwin or linux.


ACK.

I think the only sane way to handle this is to put the read in a loop
and realloc the buffer as needed as long as the read continues to return
'>0'.


Ok.

It's obviously pretty racy to get the size of the file and then expect that
you'll be able to read in exactly that many bytes.


ACK.

Revised patch attached. I'm wondering, tho, about the advisability of using alloca'ed (rather than malloc'ed) memory to hold the contents of a file of unrestricted size. Aren't there limits on the available space within a single stack frame? Does it make sense (is it legal) to alloca 40k or 50k or 500k? of memory?

Granted, most /var/lib/alternatives files are going to be fairly small (< 1k) but there's no guarantee of that...

--
Chuck


diff -urN -x .build -x .inst -x .sinst -x .gmo -x .mo alternatives-1.3.20a-orig/alternatives.c alternatives-1.3.20a/alternatives.c
--- alternatives-1.3.20a-orig/alternatives.c	2005-06-25 21:55:02.000000000 -0400
+++ alternatives-1.3.20a/alternatives.c	2005-11-06 13:36:00.468750000 -0500
@@ -16,6 +16,15 @@
 
 #define	FLAGS_TEST	(1 << 0)
 #define	FLAGS_VERBOSE	(1 << 1)
+#if defined(O_BINARY)
+# define _O_BINARY		O_BINARY
+# define FOPEN_WRITE_MODE	"wb"
+# define FOPEN_READ_MODE	"rb"
+#else
+# define _O_BINARY		0
+# define FOPEN_WRITE_MODE	"w"
+# define FOPEN_READ_MODE	"r"
+#endif
 
 #define FL_TEST(flags)	    ((flags) & FLAGS_TEST)
 #define FL_VERBOSE(flags)   ((flags) & FLAGS_VERBOSE)
@@ -115,16 +124,30 @@
 
 char * parseLine(char ** buf) {
     char * start = *buf;
-    char * end;
+    char * end1 = *buf;
+    char * end2;
 
     if (!*buf || !**buf) return NULL;
 
-    end = strchr(start, '\n');
-    if (!end) {
-	*buf = start + strlen(start);
-    } else {
-	*buf = end + 1;
-	*end = '\0';
+    while (*end1 && (*end1 != '\n') && (*end1 != '\r'))
+       end1++;
+
+    end2 = end1;
+    while (*end2 && (*end2 == '\r')) /* only walk past '\r', NOT '\n' */
+    {
+       *end2 = '\0';
+       end2++;
+    }
+
+    /* ensures this parseLine() only consumes ONE '\n' */
+    if (*end2 == '\n')
+    {
+       *buf = end2 + 1;
+       *end2 = '\0';
+    }
+    else
+    {
+       *buf = end2;
     }
 
     while (isspace(*start) && *start) start++;
@@ -139,6 +162,7 @@
     int i;
     struct stat sb;
     char * buf;
+    char * bufp;
     char * end;
     char * line;
     struct {
@@ -147,6 +171,9 @@
     } * groups = NULL;
     int numGroups = 0;
     char linkBuf[1024];
+    size_t curBufSz;
+    size_t totalBytesRead;
+    size_t numBytesRead;
 
     set->alts = NULL;
     set->numAlts = 0;
@@ -160,23 +187,44 @@
     if (FL_VERBOSE(flags))
 	printf(_("reading %s\n"), path);
 
-    if ((fd = open(path, O_RDONLY)) < 0) {
+    if ((fd = open(path, O_RDONLY | _O_BINARY)) < 0) {
 	if (errno == ENOENT) return 3;
 	fprintf(stderr, _("failed to open %s: %s\n"), path,
 	        strerror(errno));
 	return 1;
     }
 
-    fstat(fd, &sb);
-    buf = alloca(sb.st_size + 1);
-    if (read(fd, buf, sb.st_size) != sb.st_size) {
-	close(fd);
+    curBufSz = 16;
+    totalBytesRead = 0;
+    buf = alloca(curBufSz + 1);
+    bufp = buf + totalBytesRead;
+    while ( (numBytesRead = read(fd, bufp, curBufSz - totalBytesRead)) > 0 )
+    {
+       char* newBuf;
+       size_t newBufSz;
+
+       totalBytesRead += numBytesRead;
+
+       if (curBufSz < 4096)
+          newBufSz = curBufSz * 2;
+       else
+          newBufSz = curBufSz + 4096;
+
+       newBuf = alloca(newBufSz + 1);
+       memcpy(newBuf, buf, curBufSz);
+       curBufSz = newBufSz;
+       buf = newBuf; /* alloca'ed memory automatically reclaimed */
+       bufp = buf + totalBytesRead;
+    }
+    if (numBytesRead < 0)
+    {
+        close(fd);
 	fprintf(stderr, _("failed to read %s: %s\n"), path,
 	        strerror(errno));
 	return 1;
     }
     close(fd);
-    buf[sb.st_size] = '\0';
+    buf[curBufSz] = '\0';
 
     line = parseLine(&buf);
     if (!line) {
@@ -414,7 +462,7 @@
     if (FL_TEST(flags))
 	fd = dup(1);
     else
-	fd = open(path, O_RDWR | O_CREAT | O_EXCL, 0644);
+	fd = open(path, O_RDWR | O_CREAT | O_EXCL | _O_BINARY, 0644);
 
     if (fd < 0) {
 	if (errno == EEXIST) 
@@ -425,7 +473,7 @@
 	return 1;
     }
 
-    f = fdopen(fd, "w");
+    f = fdopen(fd, FOPEN_WRITE_MODE); 
     fprintf(f, "%s\n", set->mode == AUTO ? "auto" : "manual");
     fprintf(f, "%s\n", set->alts[0].master.facility);
     for (i = 0; i < set->alts[0].numSlaves; i++) {

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]