On 02/09/2008 12:00 AM, Dirk-Willem van Gulik wrote: > > On Feb 8, 2008, at 11:56 PM, Ruediger Pluem wrote: > >> There is a slightly different usage of ap_cache_cacheable_hdrs_out in >> mod_cache itself. So I would prefer: > > Below makes sese -- but is the use in mod_cache correct ?
IMHO yes. We are in the cache save filter there and we had a conditional request to the backend with our *own* conditionals, because we thought that our cached copy might not be fresh. Now that we have received a 304 we need to merge the headers of the 304 response with the cached headers before we write them back to the cache (see comment above the section, why we cannot simple replace the cached headers with the ones of the 304). We *cannot* set the content-type here as the 304 may not contain it. We get the content-type back from the cached headers. > >> 1. Rename ap_cache_cacheable_hdrs_out to ap_cache_cacheable_hdrs and >> keep it public. >> 2. Create ap_cache_cacheable_hdrs_out / ap_cache_cacheable_hdrs_in as >> proposed by you >> and add them to the API. >> >> I think we can only do this change on trunk as it would require a >> major bump. >> Ok, formally mod_cache.h is not public, but I agree with your proposal >> that it *should* >> be public and so we should not rename ap_cache_cacheable_hdrs_out to >> ap_cache_cacheable_hdrs >> without a major bump IMHO. > > Or we keep the old slightly odd names. I currently see no essential need for these changes to be backportable. So I would prefer cleaning up the names. But there might be a compromise: 1. Make ap_cache_cacheable_hdrs a macro that is defined as ap_cache_cacheable_hdrs_out 2. Leave ap_cache_cacheable_hdrs_out as is. 3. Create ap_cache_cacheable_headers_out / ap_cache_cacheable_headers_in This should only require a minor bump. Regards Rüdiger
