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);
 
+    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);
 
+    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);


Reply via email to