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

[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

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

[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

[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