Re: svn commit: r620489 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c

[Ruediger Pluem]

On 02/11/2008 03:27 PM, [EMAIL PROTECTED] wrote:

Author: dirkx
Date: Mon Feb 11 06:27:34 2008
New Revision: 620489

URL: http://svn.apache.org/viewvc?rev=620489&view=rev
Log:
Return a little bit more error information when, say a disk is full or 
something gets write protected. Note that in some cases mod_cache.c will_also_ 
log a 'cache: store_headers failed' subsequently.

Modified:
    httpd/httpd/trunk/modules/cache/mod_disk_cache.c

Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?rev=620489&r1=620488&r2=620489&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Mon Feb 11 06:27:34 2008

@@ -846,13 +852,23 @@
                 dobj->prefix = NULL;
             }

- mkdir_structure(conf, dobj->hdrsfile, r->pool);

+            rv = mkdir_structure(conf, dobj->hdrsfile, r->pool);
+
+            if (rv != APR_SUCCESS) {
+                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+                    "disk_cache: could not create directory path to %s",
+                    dobj->hdrsfile);
+                return rv;
+            }

This non fatal here and may happen due to races with htcachelean. So I wouldn't
check the return value here and let it fail later in safe_file_rename if it
really doesn't work.

rv = apr_file_mktemp(&dobj->tfd, dobj->tempfile,

                                  APR_CREATE | APR_WRITE | APR_BINARY | 
APR_EXCL,
                                  r->pool);

if (rv != APR_SUCCESS) {

+                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+                    "disk_cache: could not create temp file %s",
+                    dobj->tempfile);
                 return rv;
             }

@@ -877,7 +893,6 @@

                 ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
                     "disk_cache: rename tempfile to varyfile failed: %s -> %s",
                     dobj->tempfile, dobj->hdrsfile);
-                apr_file_remove(dobj->tempfile, r->pool);

Why shouldn't we remove the tempfile in this case?

                 return rv;
             }

@@ -960,7 +987,13 @@
      */
     rv = apr_file_remove(dobj->hdrsfile, r->pool);
     if (rv != APR_SUCCESS) {
-        mkdir_structure(conf, dobj->hdrsfile, r->pool);
+        rv = mkdir_structure(conf, dobj->hdrsfile, r->pool);
+        if (rv != APR_SUCCESS) {
+            ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server,
+                     "disk_cache: creating directories for hdrsfile %s failed",
+                     dobj->hdrsfile);
+            return rv;
+        }

This non fatal here and may happen due to races with htcachelean. So I wouldn't
check the return value here and let it fail later in safe_file_rename if it
really doesn't work.

     }

rv = safe_file_rename(conf, dobj->tempfile, dobj->hdrsfile, r->pool);

Regards

Rüdiger