httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian J. France" <br...@brianfrance.com>
Subject Re: mod_dav_fs does not check for return value on stream_close
Date Thu, 05 Apr 2012 00:11:44 GMT
Yes, you are correct.  Looks like my merge from work code to httpd svn didn't fully work.

We have been running this patch for a week or so with no issues.

Brian


On Apr 4, 2012, at 6:24 PM, Graham Leggett wrote:

> On 15 Mar 2012, at 3:56 PM, Brian J. France wrote:
> 
>> Could somebody review the patch below for 2.2, 2.4, and trunk?
>> 
>> A better error message could be sent, but I am more worried about how the return
will effect the code after it.
>> 
>> I am thinking the file needs to be removed either via a apr_file_remove call or:
>> 
>> apr_pool_cleanup_kill(stream->p, stream, tmpfile_cleanup);
>> 
>> call, but I don't know the code well enough to know which is right and 2.4/trunk
adds even more complexity compared to 2.2.x.
>> 
>> Thoughts/Comments?
>> 
>> Thanks,
>> 
>> Brian
>> 
>> - I am still getting more details why close is failing, but for some reason it is
happening enough to cause issues. (responding 200, but no file)
>> 
>> -----
>> 
>> 2.2:
>> 
>> Index: modules/dav/fs/repos.c
>> ===================================================================
>> --- modules/dav/fs/repos.c	(revision 1300964)
>> +++ modules/dav/fs/repos.c	(working copy)
>> @@ -881,6 +881,10 @@
>> {
>>    apr_file_close(stream->f);
> 
> Would be above not be
> 
> status = apr_file_close(stream->f);
> 
>> 
>> +    if (status != APR_SUCCESS) {
>> +	return dav_new_error(stream->p, MAP_IO2HTTP(status), 0, "There was a problem
closing the stream");
>> +    }
>> +
>>    if (!commit) {
>>        if (apr_file_remove(stream->pathname, stream->p) != APR_SUCCESS) {
>>            /* ### use a better description? */
>> 
>> 
>> 2.24 and trunk:
>> 
>> 
>> Index: modules/dav/fs/repos.c
>> ===================================================================
>> --- modules/dav/fs/repos.c	(revision 1300964)
>> +++ modules/dav/fs/repos.c	(working copy)
>> @@ -970,6 +970,10 @@
>> 
>>    apr_file_close(stream->f);
> 
> Same with this one.
> 
>> 
>> +    if (status != APR_SUCCESS) {
>> +	return dav_new_error(stream->p, MAP_IO2HTTP(status), 0, "There was a problem
closing the stream");
>> +    }
>> +
>>    if (!commit) {
>>        if (stream->temppath) {
>>            apr_pool_cleanup_run(stream->p, stream, tmpfile_cleanup);
>> 
>> 
> 
> Regards,
> Graham
> --
> 


Mime
View raw message