Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com
Message-ID: <436E514E.7050107@cwilson.fastmail.fm>
Date: Sun, 06 Nov 2005 13:54:06 -0500
From: Charles Wilson <cygwin AT cwilson DOT fastmail DOT fm>
User-Agent: Mozilla Thunderbird 1.0.6 (Windows/20050716)
MIME-Version: 1.0
CC: cygwin AT cygwin DOT com
Subject: Re: BUG: alternatives
References: <436CF5E9 DOT 30106 AT cwilson DOT fastmail DOT fm> <001201c5e244$c9e50490$020aa8c0 AT DFW5RB41> <436D9993 DOT 7050100 AT cwilson DOT fastmail DOT fm> <20051106134339 DOT GA24549 AT trixie DOT casa DOT cgf DOT cx> <436E50DE DOT 3070405 AT cwilson DOT fastmail DOT fm>
In-Reply-To: <436E50DE.3070405@cwilson.fastmail.fm>
Content-Type: multipart/mixed;  boundary="------------090401010507000506090408"

--------------090401010507000506090408
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

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
> 


--------------090401010507000506090408
Content-Type: text/plain;
 name="alternatives-textmode-2.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="alternatives-textmode-2.patch"

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++) {


--------------090401010507000506090408
Content-Type: text/plain; charset=us-ascii

--
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/
--------------090401010507000506090408--