You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Bert Huijben <be...@qqmail.nl> on 2017/08/30 10:28:00 UTC
RE: svn commit: r1806603 - /apr/apr/trunk/file_io/win32/readwrite.c
> -----Original Message-----
> From: ivan@apache.org [mailto:ivan@apache.org]
> Sent: dinsdag 29 augustus 2017 16:59
> To: commits@apr.apache.org
> Subject: svn commit: r1806603 - /apr/apr/trunk/file_io/win32/readwrite.c
>
> Author: ivan
> Date: Tue Aug 29 14:59:11 2017
> New Revision: 1806603
>
> URL: http://svn.apache.org/viewvc?rev=1806603&view=rev
> Log:
> Minor refactoring of the Win32 file write code.
>
> * file_io/win32/readwrite.c
> (apr_file_write): Update filePtr in the asynchronous I/O mode after
> the WriteFile() call. This allows simplifying the condition by not
> checking for thefile->pipe property.
>
> Patch by: Evgeny Kotkov <evgeny.kotkov {at} visualsvn.com>
>
> Modified:
> apr/apr/trunk/file_io/win32/readwrite.c
>
> Modified: apr/apr/trunk/file_io/win32/readwrite.c
> URL:
> http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/readwrite.c?rev
> =1806603&r1=1806602&r2=1806603&view=diff
> ==========================================================
> ====================
> --- apr/apr/trunk/file_io/win32/readwrite.c (original)
> +++ apr/apr/trunk/file_io/win32/readwrite.c Tue Aug 29 14:59:11 2017
> @@ -398,6 +398,9 @@ APR_DECLARE(apr_status_t) apr_file_write
> }
> rv = WriteFile(thefile->filehand, buf, (DWORD)*nbytes, &bwrote,
> thefile->pOverlapped);
> + if (rv == APR_SUCCESS && thefile->pOverlapped) {
> + thefile->filePtr += *nbytes;
> + }
The result of WriteFile should not be checked against APR_SUCCESS, as that is not a documented Windows result code. The right constant evaluating to 0 should be used.
Bert
Re: svn commit: r1806603 - /apr/apr/trunk/file_io/win32/readwrite.c
Posted by Ivan Zhakov <iv...@apache.org>.
On 30 August 2017 at 13:57, Evgeny Kotkov <ev...@visualsvn.com> wrote:
> Bert Huijben <be...@qqmail.nl> writes:
>
>>> rv = WriteFile(thefile->filehand, buf, (DWORD)*nbytes, &bwrote,
>>> thefile->pOverlapped);
>>> + if (rv == APR_SUCCESS && thefile->pOverlapped) {
>>> + thefile->filePtr += *nbytes;
>>> + }
>>
>> The result of WriteFile should not be checked against APR_SUCCESS, as that
>> is not a documented Windows result code. The right constant evaluating to
>> 0 should be used.
>
> Unfortunately, I think that I have missed how the return value of WriteFile()
> was being checked in the last part of apr_file_write(), and both the r1806592
> and r1806603 changesets are wrong and change the behavior of the code.
>
> As these changesets were meant to be refactorings that make the code
> clearer, without altering its behavior, I think that they should be reverted
> for now.
>
> (I will prepare an alternative patch with these simplifications that doesn't
> change the behavior of the code.)
>
>
> Sorry for that,
> Evgeny Kotkov
Reverted r1806592 and r1806603 in r1806701.
--
Ivan Zhakov
Re: svn commit: r1806603 - /apr/apr/trunk/file_io/win32/readwrite.c
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Bert Huijben <be...@qqmail.nl> writes:
>> rv = WriteFile(thefile->filehand, buf, (DWORD)*nbytes, &bwrote,
>> thefile->pOverlapped);
>> + if (rv == APR_SUCCESS && thefile->pOverlapped) {
>> + thefile->filePtr += *nbytes;
>> + }
>
> The result of WriteFile should not be checked against APR_SUCCESS, as that
> is not a documented Windows result code. The right constant evaluating to
> 0 should be used.
Unfortunately, I think that I have missed how the return value of WriteFile()
was being checked in the last part of apr_file_write(), and both the r1806592
and r1806603 changesets are wrong and change the behavior of the code.
As these changesets were meant to be refactorings that make the code
clearer, without altering its behavior, I think that they should be reverted
for now.
(I will prepare an alternative patch with these simplifications that doesn't
change the behavior of the code.)
Sorry for that,
Evgeny Kotkov