You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Brian Havard <br...@kheldar.apana.org.au> on 2001/08/28 17:14:23 UTC

Re: cvs commit: apr/file_io/win32 readwrite.c seek.c

On 28 Aug 2001 01:56:09 -0000, wrowe@apache.org wrote:

>wrowe       01/08/27 18:56:09
>
>  Modified:    file_io/win32 readwrite.c seek.c
>  Log:
>    Found a very ugly reaction to using apr_file_seek(APR_CUR, -value) in
>    conjuction with buffered reads.  Thank you for toggling that case Jeff,
>    so I could shoot out this bug ;)

Did this actually fix things for you? You see the new offset calculation in 
the buffered seek is wrong (my fault). I've only tested on OS/2 but it should 
apply equally to all platforms. The fix is:

Index: os2/seek.c
===================================================================
RCS file: /home/cvs/apr/file_io/os2/seek.c,v
retrieving revision 1.18
diff -u -r1.18 seek.c
--- seek.c	2001/08/21 12:09:50	1.18
+++ seek.c	2001/08/28 15:08:37
@@ -111,7 +111,7 @@
             break;
         }
 
-        *offset = thefile->filePtr + thefile->bufpos;
+        *offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
         return rc;
     } else {
         switch (where) {
Index: unix/seek.c
===================================================================
RCS file: /home/cvs/apr/file_io/unix/seek.c,v
retrieving revision 1.23
diff -u -r1.23 seek.c
--- seek.c	2001/08/22 15:40:29	1.23
+++ seek.c	2001/08/28 15:08:37
@@ -110,7 +110,7 @@
             break;
         }
 
-        *offset = thefile->filePtr + thefile->bufpos;
+        *offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
         return rc;
     } else {
 
Index: win32/seek.c
===================================================================
RCS file: /home/cvs/apr/file_io/win32/seek.c,v
retrieving revision 1.19
diff -u -r1.19 seek.c
--- seek.c	2001/07/31 19:51:34	1.19
+++ seek.c	2001/08/28 15:08:37
@@ -115,7 +115,7 @@
                 return APR_EINVAL;
         }
 
-        *offset = thefile->filePtr + thefile->bufpos;
+        *offset = thefile->filePtr - thefile->dataRead + thefile->bufpos;
         return rc;
     } 
     else if (thefile->pOverlapped) {

-- 
 ______________________________________________________________________________
 |  Brian Havard                 |  "He is not the messiah!                   |
 |  brianh@kheldar.apana.org.au  |  He's a very naughty boy!" - Life of Brian |
 ------------------------------------------------------------------------------


Re: cvs commit: apr/file_io/win32 readwrite.c seek.c

Posted by Jeff Trawick <tr...@attglobal.net>.
Jeff Trawick <tr...@attglobal.net> writes:

> I just started playing with /error/HTTP_NOT_FOUND.html.var at
> OtherBill's suggestion.  Even with this patch, the seek back to the end
> of the de body is not going to the right place.
> 
> gotta read some code...

I know the patch is a big improvement and it may even be the perfect
fix; I'm seeing some bogosity with the seek which may be somewhere
else.

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Re: cvs commit: apr/file_io/win32 readwrite.c seek.c

Posted by Jeff Trawick <tr...@attglobal.net>.
"Brian Havard" <br...@kheldar.apana.org.au> writes:

> On Wed, 29 Aug 2001 01:14:23 +1000 (EST), Brian Havard wrote:
> 
> >On 28 Aug 2001 01:56:09 -0000, wrowe@apache.org wrote:
> >
> >>wrowe       01/08/27 18:56:09
> >>
> >>  Modified:    file_io/win32 readwrite.c seek.c
> >>  Log:
> >>    Found a very ugly reaction to using apr_file_seek(APR_CUR, -value) in
> >>    conjuction with buffered reads.  Thank you for toggling that case Jeff,
> >>    so I could shoot out this bug ;)
> >
> >Did this actually fix things for you? You see the new offset calculation in 
> >the buffered seek is wrong (my fault). I've only tested on OS/2 but it should 
> >apply equally to all platforms. The fix is:
> 
> Forget it, different bug. Just applied similar fix to the OS/2 version.
> For unix, the patch below should do it but I can't test.
> 
> 
> Index: readwrite.c
> ===================================================================
> RCS file: /home/cvs/apr/file_io/unix/readwrite.c,v
> retrieving revision 1.71
> diff -u -r1.71 readwrite.c
> --- readwrite.c	2001/08/24 12:19:01	1.71
> +++ readwrite.c	2001/08/28 16:19:02
> @@ -137,16 +137,17 @@
>          }
>          while (rv == 0 && size > 0) {
>              if (thefile->bufpos >= thefile->dataRead) {
> -                thefile->dataRead = read(thefile->filedes, thefile->buffer, APR_FILE_BUFSIZE);
> -                if (thefile->dataRead == 0) {
> +                int bytesread = read(thefile->filedes, thefile->buffer, APR_FILE_BUFSIZE);
> +                if (bytesread == 0) {
>                      thefile->eof_hit = TRUE;
>                      rv = APR_EOF;
>                      break;
>                  }
> -                else if (thefile->dataRead == -1) {
> +                else if (bytesread == -1) {
>                      rv = errno;
>                      break;
>                  }
> +                thefile->dataRead = bytesread;
>                  thefile->filePtr += thefile->dataRead;
>                  thefile->bufpos = 0;
>              }

I just started playing with /error/HTTP_NOT_FOUND.html.var at
OtherBill's suggestion.  Even with this patch, the seek back to the end
of the de body is not going to the right place.

gotta read some code...
--- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Re: cvs commit: apr/file_io/win32 readwrite.c seek.c

Posted by Brian Havard <br...@kheldar.apana.org.au>.
On Wed, 29 Aug 2001 01:14:23 +1000 (EST), Brian Havard wrote:

>On 28 Aug 2001 01:56:09 -0000, wrowe@apache.org wrote:
>
>>wrowe       01/08/27 18:56:09
>>
>>  Modified:    file_io/win32 readwrite.c seek.c
>>  Log:
>>    Found a very ugly reaction to using apr_file_seek(APR_CUR, -value) in
>>    conjuction with buffered reads.  Thank you for toggling that case Jeff,
>>    so I could shoot out this bug ;)
>
>Did this actually fix things for you? You see the new offset calculation in 
>the buffered seek is wrong (my fault). I've only tested on OS/2 but it should 
>apply equally to all platforms. The fix is:

Forget it, different bug. Just applied similar fix to the OS/2 version.
For unix, the patch below should do it but I can't test.


Index: readwrite.c
===================================================================
RCS file: /home/cvs/apr/file_io/unix/readwrite.c,v
retrieving revision 1.71
diff -u -r1.71 readwrite.c
--- readwrite.c	2001/08/24 12:19:01	1.71
+++ readwrite.c	2001/08/28 16:19:02
@@ -137,16 +137,17 @@
         }
         while (rv == 0 && size > 0) {
             if (thefile->bufpos >= thefile->dataRead) {
-                thefile->dataRead = read(thefile->filedes, thefile->buffer, APR_FILE_BUFSIZE);
-                if (thefile->dataRead == 0) {
+                int bytesread = read(thefile->filedes, thefile->buffer, APR_FILE_BUFSIZE);
+                if (bytesread == 0) {
                     thefile->eof_hit = TRUE;
                     rv = APR_EOF;
                     break;
                 }
-                else if (thefile->dataRead == -1) {
+                else if (bytesread == -1) {
                     rv = errno;
                     break;
                 }
+                thefile->dataRead = bytesread;
                 thefile->filePtr += thefile->dataRead;
                 thefile->bufpos = 0;
             }

-- 
 ______________________________________________________________________________
 |  Brian Havard                 |  "He is not the messiah!                   |
 |  brianh@kheldar.apana.org.au  |  He's a very naughty boy!" - Life of Brian |
 ------------------------------------------------------------------------------