You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by jean-frederic clere <jf...@fujitsu-siemens.com> on 2004/10/08 15:34:18 UTC

mod_deflate.c problems

Hi,

I have the following problem with mod_deflate:

+++
In file included from /usr/include/zutil.h:16,
                  from mod_deflate.c:50:
/usr/include/zlib.h:40: warning: `ZLIB_VERSION' redefined
/opt/SMAWPlus/include/zlib.h:40: warning: this is the location of the previous 
definition
In file included from /usr/include/zlib.h:34,
                  from /usr/include/zutil.h:16,
                  from mod_deflate.c:50:
+++

Does it make sense to add a test to make sure zutil.h will not conflict with the 
  libz we want to use to build mod_deflate.c?

Cheers

Jean-Frederic

Re: mod_deflate.c problems

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Saturday, October 9, 2004 10:54 AM +0100 Joe Orton <jo...@redhat.com> 
wrote:

> It's apparently an issue when you have zlib 1.2.1 installed somewhere,
> since in that version zutil.h is considered an "internal" header and is
> not installed by make install:
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=28673
>
> so that's a good motivation for not using it too.

Hmm.  How are you legitimately supposed to set OS_CODE then if you don't have 
the header?  (I know we hacked around that, but the rationale was that it was 
an installation error not to have it....)  If they truly removed it now and 
don't have a way to even get the OS_CODE #define, then yah, I guess, let's 
toss it and hard-code to 0x03.

*shrug*  -- justin

Re: mod_deflate.c problems

Posted by Joe Orton <jo...@redhat.com>.
On Fri, Oct 08, 2004 at 08:50:19AM -0700, Justin Erenkrantz wrote:
> --On Friday, October 8, 2004 3:59 PM +0100 Joe Orton <jo...@redhat.com> 
> wrote:
> 
> >zutil.h isn't really needed at all; in fact whole the gzip header could
> >be in an immortal bucket, much less overhead than jumping through the
> >psprintf code.  Try this?
> 
> I think we tried to hard-code OS_CODE to 0x03 initially and ran into some 
> type of portability problem.  That's why we were trying to set it 
> 'correctly.' Either way, it could still be an immortal bucket - that part 
> of your change is probably goodness.

I guess I can see there would have been problems from assuming zutil.h
exists and OS_CODE was defined, hence the current code, but it's hard to
imagine there are portability problems from just not using the things at
all... 

> 
> Jean-Frederic's problem just appears to be a warning because his install is 
> corrupt somehow.  It looks like he has conflicting zlib installations - so 
> I'd also expect the linker to get confused as well.  -- justin

It's apparently an issue when you have zlib 1.2.1 installed somewhere,
since in that version zutil.h is considered an "internal" header and is
not installed by make install:

http://issues.apache.org/bugzilla/show_bug.cgi?id=28673

so that's a good motivation for not using it too.

joe

Re: mod_deflate.c problems

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Friday, October 8, 2004 3:59 PM +0100 Joe Orton <jo...@redhat.com> wrote:

> zutil.h isn't really needed at all; in fact whole the gzip header could
> be in an immortal bucket, much less overhead than jumping through the
> psprintf code.  Try this?

I think we tried to hard-code OS_CODE to 0x03 initially and ran into some type 
of portability problem.  That's why we were trying to set it 'correctly.' 
Either way, it could still be an immortal bucket - that part of your change is 
probably goodness.

Jean-Frederic's problem just appears to be a warning because his install is 
corrupt somehow.  It looks like he has conflicting zlib installations - so I'd 
also expect the linker to get confused as well.  -- justin

Re: mod_deflate.c problems

Posted by Joe Orton <jo...@redhat.com>.
On Fri, Oct 08, 2004 at 03:34:18PM +0200, jean-frederic clere wrote:
> I have the following problem with mod_deflate:
>
> +++
> In file included from /usr/include/zutil.h:16,
>                  from mod_deflate.c:50:
> /usr/include/zlib.h:40: warning: `ZLIB_VERSION' redefined
> /opt/SMAWPlus/include/zlib.h:40: warning: this is the location of the 
> previous definition
> In file included from /usr/include/zlib.h:34,
>                  from /usr/include/zutil.h:16,
>                  from mod_deflate.c:50:

zutil.h isn't really needed at all; in fact whole the gzip header could
be in an immortal bucket, much less overhead than jumping through the
psprintf code.  Try this?

Index: filters/mod_deflate.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/filters/mod_deflate.c,v
retrieving revision 1.57
diff -u -r1.57 mod_deflate.c
--- filters/mod_deflate.c	23 Jul 2004 12:40:49 -0000	1.57
+++ filters/mod_deflate.c	8 Oct 2004 14:56:56 -0000
@@ -46,52 +46,6 @@
 
 #include "zlib.h"
 
-#ifdef HAVE_ZUTIL_H
-#include "zutil.h"
-#else
-/* As part of the encoding process, we must send what our OS_CODE is
- * (or so it seems based on what I can tell of how gzip encoding works).
- *
- * zutil.h is not always included with zlib distributions (it is a private
- * header), so this is straight from zlib 1.1.3's zutil.h.
- */
-#ifdef OS2
-#define OS_CODE  0x06
-#endif
-
-#ifdef WIN32 /* Window 95 & Windows NT */
-#define OS_CODE  0x0b
-#endif
-
-#if defined(VAXC) || defined(VMS)
-#define OS_CODE  0x02
-#endif
-
-#ifdef AMIGA
-#define OS_CODE  0x01
-#endif
-
-#if defined(ATARI) || defined(atarist)
-#define OS_CODE  0x05
-#endif
-
-#if defined(MACOS) || defined(TARGET_OS_MAC)
-#define OS_CODE  0x07
-#endif
-
-#ifdef __50SERIES /* Prime/PRIMOS */
-#define OS_CODE  0x0F
-#endif
-
-#ifdef TOPS20
-#define OS_CODE  0x0a
-#endif
-
-#ifndef OS_CODE
-#define OS_CODE  0x03  /* assume Unix */
-#endif
-#endif
-
 static const char deflateFilterName[] = "DEFLATE";
 module AP_MODULE_DECLARE_DATA deflate_module;
 
@@ -106,6 +60,21 @@
     char *note_output_name;
 } deflate_filter_config;
 
+/* RFC 1952 Section 2.3 dictates the gzip header:
+ *
+ * +---+---+---+---+---+---+---+---+---+---+
+ * |ID1|ID2|CM |FLG|     MTIME     |XFL|OS |
+ * +---+---+---+---+---+---+---+---+---+---+
+ */
+static const char gzip_header[10] = 
+{ '\037', '\213', Z_DEFLATED, 0,
+  0, 0, 0, 0, /* mtime */
+  0, 0x03 /* Unix OS_CODE */
+};
+
+/* magic header */
+static const char deflate_magic[2] = { '\037', '\213' };
+
 /* windowsize is negative to suppress Zlib header */
 #define DEFAULT_COMPRESSION Z_DEFAULT_COMPRESSION
 #define DEFAULT_WINDOWSIZE -15
@@ -236,9 +205,6 @@
     return NULL;
 }
 
-/* magic header */
-static char deflate_magic[2] = { '\037', '\213' };
-
 typedef struct deflate_ctx_t
 {
     z_stream stream;
@@ -269,7 +235,7 @@
      * we're in better shape.
      */
     if (!ctx) {
-        char *buf, *token;
+        char *token;
         const char *encoding;
 
         /* only work on main request/no subrequests */
@@ -419,22 +385,9 @@
             return ap_pass_brigade(f->next, bb);
         }
 
-        /* RFC 1952 Section 2.3 dictates the gzip header:
-         *
-         * +---+---+---+---+---+---+---+---+---+---+
-         * |ID1|ID2|CM |FLG|     MTIME     |XFL|OS |
-         * +---+---+---+---+---+---+---+---+---+---+
-         *
-         * If we wish to populate in MTIME (as hinted in RFC 1952), do:
-         * putLong(date_array, apr_time_now() / APR_USEC_PER_SEC);
-         * where date_array is a char[4] and then print date_array in the
-         * MTIME position.  WARNING: ENDIANNESS ISSUE HERE.
-         */
-        buf = apr_psprintf(r->pool, "%c%c%c%c%c%c%c%c%c%c", deflate_magic[0],
-                           deflate_magic[1], Z_DEFLATED, 0 /* flags */,
-                           0, 0, 0, 0 /* 4 chars for mtime */,
-                           0 /* xflags */, OS_CODE);
-        e = apr_bucket_pool_create(buf, 10, r->pool, f->c->bucket_alloc);
+        /* add immortal gzip header */
+        e = apr_bucket_immortal_create(gzip_header, sizeof gzip_header,
+                                       f->c->bucket_alloc);
         APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
 
         /* If the entire Content-Encoding is "identity", we can replace it. */