You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2000/11/05 22:47:47 UTC
APR patch: Win32 fixes
Two nits here.
* apr_full_read would loop infinitely if the file was smaller than
the buffer, because read_with_timeout wouldn't return APR_EOF.
* apr_read wouldn't read because rv wasn't initialised.
* apr/file_io/win32/readwrite.c (read_with_timeout):
Successful ReadFile with 0 bytes read indicates end-of-file.
(apr_read): Initialise `rv' before use.
Index: readwrite.c
===================================================================
RCS file: /home/cvspublic/apache-2.0/src/lib/apr/file_io/win32/readwrite.c,v
retrieving revision 1.47
diff -u -r1.47 readwrite.c
--- readwrite.c 2000/10/16 06:04:40 1.47
+++ readwrite.c 2000/11/05 22:39:09
@@ -139,7 +139,10 @@
rv = APR_SUCCESS; /* APR_EOF? */
}
} else {
- rv = APR_SUCCESS;
+ if (*nbytes == 0) /* OK and 0 bytes read ==> end of file */
+ rv = APR_EOF;
+ else
+ rv = APR_SUCCESS;
}
return rv;
}
@@ -240,6 +243,7 @@
thefile->direction = 1;
}
+ rv = 0;
while (rv == 0 && size > 0) {
if (thefile->bufpos == APR_FILE_BUFSIZE) // write buffer is full
rv = apr_flush(thefile);
--
Brane �ibej
home: <br...@xbc.nu> http://www.xbc.nu/brane/
work: <br...@hermes.si> http://www.hermes-softlab.com/
ACM: <br...@acm.org> http://www.acm.org/
Re: APR patch: Win32 fixes
Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:
> Is this necessary? I know that Bill Rowe was considering torching the .def
> file altogether. (I forget the reason why, but he mentioned this at
> ApacheCon)
>
> Is this needed because we aren't using APR_DECLARE() throughout the APR
> header files? e.g. to get it exported, this change or the header change
> needs to happen?
Yes, that's right.
Oh, killing off the .def file would be fine, iff all the public APIs are
explicitly exported.
Anyway, I don't need it for Subversion right now.
--
Brane �ibej
home: <br...@xbc.nu> http://www.xbc.nu/brane/
work: <br...@hermes.si> http://www.hermes-softlab.com/
ACM: <br...@acm.org> http://www.acm.org/
Re: APR patch: Win32 fixes
Posted by Greg Stein <gs...@lyra.org>.
Is this necessary? I know that Bill Rowe was considering torching the .def
file altogether. (I forget the reason why, but he mentioned this at
ApacheCon)
Is this needed because we aren't using APR_DECLARE() throughout the APR
header files? e.g. to get it exported, this change or the header change
needs to happen?
Cheers,
-g
On Sun, Nov 05, 2000 at 11:53:09PM +0100, Branko Cibej wrote:
> Oh, forgot this one:
>
>
> * apr/aprlib.def: Add `apr_sleep'.
>
>
> Index: aprlib.def
> ===================================================================
> RCS file: /home/cvspublic/apache-2.0/src/lib/apr/aprlib.def,v
> retrieving revision 1.36
> diff -u -r1.36 aprlib.def
> --- aprlib.def 2000/10/23 17:21:10 1.36
> +++ aprlib.def 2000/11/05 22:50:32
> @@ -139,6 +139,7 @@
> apr_ctime @136
> apr_rfc822_date @137
> apr_strftime @138
> + apr_sleep @139
> ;
> ;
> ;
>
> --
> Brane Cibej
> home: <br...@xbc.nu> http://www.xbc.nu/brane/
> work: <br...@hermes.si> http://www.hermes-softlab.com/
> ACM: <br...@acm.org> http://www.acm.org/
>
--
Greg Stein, http://www.lyra.org/
Re: APR patch: Win32 fixes
Posted by Branko Čibej <br...@xbc.nu>.
Oh, forgot this one:
* apr/aprlib.def: Add `apr_sleep'.
Index: aprlib.def
===================================================================
RCS file: /home/cvspublic/apache-2.0/src/lib/apr/aprlib.def,v
retrieving revision 1.36
diff -u -r1.36 aprlib.def
--- aprlib.def 2000/10/23 17:21:10 1.36
+++ aprlib.def 2000/11/05 22:50:32
@@ -139,6 +139,7 @@
apr_ctime @136
apr_rfc822_date @137
apr_strftime @138
+ apr_sleep @139
;
;
;
--
Brane �ibej
home: <br...@xbc.nu> http://www.xbc.nu/brane/
work: <br...@hermes.si> http://www.hermes-softlab.com/
ACM: <br...@acm.org> http://www.acm.org/
Re: APR patch: Win32 fixes
Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:
> On Sun, Nov 05, 2000 at 11:47:47PM +0100, Branko Cibej wrote:
>
>> Two nits here.
>>
>> * apr_full_read would loop infinitely if the file was smaller than
>> the buffer, because read_with_timeout wouldn't return APR_EOF.
>> * apr_read wouldn't read because rv wasn't initialised.
>>
>>
>> * apr/file_io/win32/readwrite.c (read_with_timeout):
>> Successful ReadFile with 0 bytes read indicates end-of-file.
>> (apr_read): Initialise `rv' before use.
>
>
> This looks good except that you didn't compensate for the new return value
> from read_with_timeout() on the "buffered" case in apr_read(). I tweaked
> that one and am checking it in now...
Oops ... my bad. Thanks.
--
Brane �ibej
home: <br...@xbc.nu> http://www.xbc.nu/brane/
work: <br...@hermes.si> http://www.hermes-softlab.com/
ACM: <br...@acm.org> http://www.acm.org/
Re: APR patch: Win32 fixes
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Nov 05, 2000 at 11:47:47PM +0100, Branko Cibej wrote:
> Two nits here.
>
> * apr_full_read would loop infinitely if the file was smaller than
> the buffer, because read_with_timeout wouldn't return APR_EOF.
> * apr_read wouldn't read because rv wasn't initialised.
>
>
> * apr/file_io/win32/readwrite.c (read_with_timeout):
> Successful ReadFile with 0 bytes read indicates end-of-file.
> (apr_read): Initialise `rv' before use.
This looks good except that you didn't compensate for the new return value
from read_with_timeout() on the "buffered" case in apr_read(). I tweaked
that one and am checking it in now...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/