You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@apache.org on 2001/08/28 03:56:09 UTC
cvs commit: apr/file_io/win32 readwrite.c seek.c
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 ;)
Revision Changes Path
1.61 +9 -6 apr/file_io/win32/readwrite.c
Index: readwrite.c
===================================================================
RCS file: /home/cvs/apr/file_io/win32/readwrite.c,v
retrieving revision 1.60
retrieving revision 1.61
diff -u -r1.60 -r1.61
--- readwrite.c 2001/06/20 00:56:02 1.60
+++ readwrite.c 2001/08/28 01:56:09 1.61
@@ -159,7 +159,7 @@
APR_DECLARE(apr_status_t) apr_file_read(apr_file_t *thefile, void *buf, apr_size_t *len)
{
- apr_size_t rv;
+ apr_status_t rv;
DWORD bytes_read = 0;
if (*len <= 0) {
@@ -196,16 +196,19 @@
rv = 0;
while (rv == 0 && size > 0) {
if (thefile->bufpos >= thefile->dataRead) {
+ apr_size_t read;
rv = read_with_timeout(thefile, thefile->buffer,
- APR_FILE_BUFSIZE, &thefile->dataRead);
- if (thefile->dataRead == 0) {
+ APR_FILE_BUFSIZE, &read);
+ if (read == 0) {
if (rv == APR_EOF)
thefile->eof_hit = TRUE;
break;
}
-
- thefile->filePtr += thefile->dataRead;
- thefile->bufpos = 0;
+ else {
+ thefile->dataRead = read;
+ thefile->filePtr += thefile->dataRead;
+ thefile->bufpos = 0;
+ }
}
blocksize = size > thefile->dataRead - thefile->bufpos ? thefile->dataRead - thefile->bufpos : size;
1.20 +1 -1 apr/file_io/win32/seek.c
Index: seek.c
===================================================================
RCS file: /home/cvs/apr/file_io/win32/seek.c,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -r1.19 -r1.20
--- seek.c 2001/07/31 19:51:34 1.19
+++ seek.c 2001/08/28 01:56:09 1.20
@@ -82,7 +82,7 @@
else
rc = APR_SUCCESS;
if (rc == APR_SUCCESS)
- thefile->bufpos = thefile->dataRead = 0;
+ thefile->eof_hit = thefile->bufpos = thefile->dataRead = 0;
}
return rc;
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 |
------------------------------------------------------------------------------
Re: cvs commit: apr/file_io/win32 readwrite.c seek.c
Posted by Brian Havard <br...@kheldar.apana.org.au>.
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 |
------------------------------------------------------------------------------