You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by nd...@apache.org on 2003/01/01 21:31:37 UTC

cvs commit: httpd-2.0/modules/filters mod_deflate.c

nd          2003/01/01 12:31:37

  Modified:    modules/filters mod_deflate.c
  Log:
  The patch allows the user to log the accurate filter input and
  output byte count, instead of only the rounded compression ratio.
  The DeflateFilterNote directive will be extended as follows:
  
  DeflateFilterNote [type] name
  
  type can be one of "input", "output" or "ratio". "ratio" is assumed if the
  type is omitted (backwards compatible).
  
  Revision  Changes    Path
  1.27      +31 -17    httpd-2.0/modules/filters/mod_deflate.c
  
  Index: mod_deflate.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/filters/mod_deflate.c,v
  retrieving revision 1.26
  retrieving revision 1.27
  diff -u -r1.26 -r1.27
  --- mod_deflate.c	14 Nov 2002 19:22:26 -0000	1.26
  +++ mod_deflate.c	1 Jan 2003 20:31:37 -0000	1.27
  @@ -129,7 +129,9 @@
       int windowSize;
       int memlevel;
       apr_size_t bufferSize;
  -    char *noteName;
  +    char *noteRatioName;
  +    char *noteInputName;
  +    char *noteOutputName;
   } deflate_filter_config;
   
   /* windowsize is negative to suppress Zlib header */
  @@ -202,11 +204,26 @@
       return NULL;
   }
   static const char *deflate_set_note(cmd_parms *cmd, void *dummy,
  -                                    const char *arg)
  +                                    const char *arg1, const char *arg2)
   {
       deflate_filter_config *c = ap_get_module_config(cmd->server->module_config,
                                                       &deflate_module);
  -    c->noteName = apr_pstrdup(cmd->pool, arg);
  +    
  +    if (arg2 == NULL) {
  +        c->noteRatioName = apr_pstrdup(cmd->pool, arg1);
  +    }
  +    else if (!strcasecmp(arg1, "ratio")) {
  +        c->noteRatioName = apr_pstrdup(cmd->pool, arg2);
  +    }
  +    else if (!strcasecmp(arg1, "input")) {
  +        c->noteInputName = apr_pstrdup(cmd->pool, arg2);
  +    }
  +    else if (!strcasecmp(arg1, "output")) {
  +        c->noteOutputName = apr_pstrdup(cmd->pool, arg2);
  +    }
  +    else {
  +        return apr_psprintf(cmd->pool, "Unknown note type %s", arg1);
  +    }
   
       return NULL;
   }
  @@ -239,6 +256,11 @@
       apr_bucket_brigade *bb, *proc_bb;
   } deflate_ctx;
   
  +#define LeaveNote(type, value) \
  +    if (c->note##type##Name) \
  +        apr_table_setn(r->notes, c->note##type##Name, \
  +                       (ctx->stream.total_in > 0) ? (value) : "-")
  +
   static apr_status_t deflate_out_filter(ap_filter_t *f,
                                          apr_bucket_brigade *bb)
   {
  @@ -433,19 +455,11 @@
                             "Zlib: Compressed %ld to %ld : URL %s",
                             ctx->stream.total_in, ctx->stream.total_out, r->uri);
   
  -            if (c->noteName) {
  -                if (ctx->stream.total_in > 0) {
  -                    int total;
  -
  -                    total = ctx->stream.total_out * 100 / ctx->stream.total_in;
  -
  -                    apr_table_setn(r->notes, c->noteName,
  -                                   apr_itoa(r->pool, total));
  -                }
  -                else {
  -                    apr_table_setn(r->notes, c->noteName, "-");
  -                }
  -            }
  +            LeaveNote(Input, apr_off_t_toa(r->pool, ctx->stream.total_in));
  +            LeaveNote(Output, apr_off_t_toa(r->pool, ctx->stream.total_out));
  +            LeaveNote(Ratio, apr_itoa(r->pool, (int)
  +                                      (ctx->stream.total_out * 100 /
  +                                       ctx->stream.total_in)));
   
               deflateEnd(&ctx->stream);
   
  @@ -775,7 +789,7 @@
   }
   
   static const command_rec deflate_filter_cmds[] = {
  -    AP_INIT_TAKE1("DeflateFilterNote", deflate_set_note, NULL, RSRC_CONF,
  +    AP_INIT_TAKE12("DeflateFilterNote", deflate_set_note, NULL, RSRC_CONF,
                     "Set a note to report on compression ratio"),
       AP_INIT_TAKE1("DeflateWindowSize", deflate_set_window_size, NULL,
                     RSRC_CONF, "Set the Deflate window size (1-15)"),
  
  
  

Re: cvs commit: httpd-2.0/modules/filters mod_deflate.c

Posted by André Malo <nd...@perlig.de>.
* Justin Erenkrantz wrote:

> We don't use mixed case for any definitions, and we require braces at
> all times, and for macros longer than one line, we wrap it with a do
> {} while (0) clause so that some compilers are a bit happier.

ah ok, thanks.

<snip>

> All of that said, I'm not really sure this code even merits being a
> macro.  I'd rather see the conditional execution clear at the scope
> where LeaveNote is called rather than hidden in its definition.
> Yeah, it's slightly repetitive, but I'm not really buying what the
> macro is getting us here.  IMHO, there should be an extremely high bar
> to creating a macro.

My thought was mainly to get some (perhaps subtle) semantics and better 
readability. But I've no problem to put this directly into the code; it's 
not so much, that one would loose the overview.

...tomorrow, if the time is ok ;-)

nd
-- 
s;.*;aaaaaoaaaoaaaaooooaaoaaaomaaaa:a:alataa:aaoat:a:a:a
maoaa:a:laoata:a:oia:a:o:a:m:a:o:alaoooat:aaool:aaoaa
matooololaaatoto:aaa:o:a:o:m;;s:\s:\::g;y;mailto:;
\40\51/\134\137|ndparker <nd...@perlig.de>;;print;

RE: cvs commit: httpd-2.0/modules/filters mod_deflate.c

Posted by Bill Stoddard <bi...@wstoddard.com>.
> IMHO, there should be an extremely high bar
> to creating a macro.  -- justin

I fricking hate macros but there are times they are useful. This isn't one of
them IMHO.

Bill


Re: cvs commit: httpd-2.0/modules/filters mod_deflate.c

Posted by Justin Erenkrantz <je...@apache.org>.
--On Wednesday, January 1, 2003 8:31 PM +0000 nd@apache.org wrote:

>   @@ -239,6 +256,11 @@
>        apr_bucket_brigade *bb, *proc_bb;
>    } deflate_ctx;
>
>   +#define LeaveNote(type, value) \
>   +    if (c->note##type##Name) \
>   +        apr_table_setn(r->notes, c->note##type##Name, \
>   +                       (ctx->stream.total_in > 0) ? (value) : 
"-")

We don't use mixed case for any definitions, and we require braces at
all times, and for macros longer than one line, we wrap it with a do
{} while (0) clause so that some compilers are a bit happier.

So, I would probably suggest something like:

#define leave_note(type, value) \
do { \
    if (c->note##type##Name) { \
        apr_table_setn(r->notes, c->note##type##Name, \
                       (ctx->stream.total_in > 0) ? (value) : "-") \
    } \
} while (0)

>  +            LeaveNote(Input, apr_off_t_toa(r->pool, 
ctx->stream.total_in));
>  +            LeaveNote(Output, apr_off_t_toa(r->pool, 
ctx-stream.total_out));
>  +            LeaveNote(Ratio, apr_itoa(r->pool, (int)
>  +                                      (ctx->stream.total_out * 
100 / ctx->stream.total_in)));

All of that said, I'm not really sure this code even merits being a
macro.  I'd rather see the conditional execution clear at the scope
where LeaveNote is called rather than hidden in its definition.
Yeah, it's slightly repetitive, but I'm not really buying what the
macro is getting us here.  IMHO, there should be an extremely high bar
to creating a macro.  -- justin