On 21.02.2011 23:55, Blair Zajac wrote:
> On 2/9/11 10:30 AM, Blair Zajac wrote:
>> On 2/9/11 1:38 AM, John Szakmeister wrote:
>>> On Mon, Feb 7, 2011 at 4:26 PM, Blair Zajac<blair_at_orcaware.com> wrote:
>>>> [I sent this to dev_at_apr.apache.org but haven't received a response.
>>>> Thread
>>>> here:
>>>> http://mail-archives.apache.org/mod_mbox/apr-dev/201102.mbox/%3CF7B1928D-D32F-48DD-B8D9-80B26906AE51@orcaware.com%3E
>>>>
>>>>
>>>> . Given the importance of writing complete files for svn, could
>>>> somebody
>>>> take a look and see if this is a valid issue].
>>>>
>>>> I was looking at apr_file_flush() and think I found a bug where if
>>>> write()
>>>> doesn't do a full write, then the apr_file_t will destroy and
>>>> buffered data
>>>> because it sets its internal buffer position back to 0.
>>>
>>> Yeah, that looks like a bug.
>>>
>>>> Here's a patch for this:
>>>>
>>>> Index: file_io/unix/readwrite.c
>>>> ===================================================================
>>>> --- file_io/unix/readwrite.c (revision 1067340)
>>>> +++ file_io/unix/readwrite.c (working copy)
>>>> @@ -409,7 +409,11 @@
>>>> rv = errno;
>>>> } else {
>>>> thefile->filePtr += written;
>>>> - thefile->bufpos = 0;
>>>> + if (written != thefile->bufpos)
>>>> + memmove(thefile->buffer,
>>>> + thefile->buffer + written,
>>>> + thefile->bufpos - written);
>>>> + thefile->bufpos -= written;
>>>> }
>>>> }
>>>>
>>>> Beyond this, there's no a mechanism to report that all the buffered
>>>> data
>>>> didn't get into the file. Perhaps apr_file_flush() should loop
>>>> until the
>>>> entire buffer is written or it gets a non-EINTR error?
>>>
>>> I think it you're right, it should loop around. Good catch!
>>
>> Thanks!
>>
>> Who here can commit to apr?
>
> Stefan Fritsch committed a looping solution in r1073142 (trunk),
> r1073145 (1.5.x), r1073146 (1.4.x).
The looping solution looks better than the memmove.
Are we ready to declare apr-0.9 out of bounds? Our code still nominally
supports that version, but I thin we should move forward, compatibility
guidelines notwithstanding.
If there's disagreement, I can probably port that fix to apr-0.9 ... but
there's not likely to be another 0.9.x release, ever.
-- Brane
Received on 2011-02-22 01:55:24 CET