httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r620489 - /httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Date Mon, 11 Feb 2008 20:02:53 GMT


On 02/11/2008 03:27 PM, dirkx@apache.org 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


Mime
View raw message