httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jerenkra...@apache.org>
Subject Re: cvs commit: httpd-2.0/modules/filters mod_deflate.c
Date Thu, 02 Jan 2003 03:56:08 GMT
--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

Mime
View raw message