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<bl...@orcaware.com> wrote: >>>> [I sent this to d...@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-80b26906a...@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
