You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/02/13 07:13:05 UTC
brigade/bucket insertion macros
Is anybody else getting sick of the APR_BRIGADE_INSERT_foo and
APR_BUCKET_INSERT_foo macros in that you have to use a bazillion sequences
like this?
e = apr_bucket_file_create(fd, 0, r->finfo.size);
APR_BRIGADE_INSERT_HEAD(bb, e);
e = apr_bucket_eos_create();
APR_BRIGADE_INSERT_TAIL(bb, e);
The problem driving this, obviously, stems from macro expansion. But these
sequences are very, very common. So why not let the macros handle it for
you, as with the appended patch? (For brevity, I've just included a sample
snippet of the patch inline and the entire patch is at the end.) I've
modified just the bucket/brigade macros, although this could just as easily
be accomplished all the way down at the ring level... I'm just being
conservative.
-#define APR_BRIGADE_INSERT_HEAD(b, e) \
- APR_RING_INSERT_HEAD(&(b)->list, (e), apr_bucket, link)
+#define APR_BRIGADE_INSERT_HEAD(b, e) do { \
+ apr_bucket *ap__b = (e); \
+ APR_RING_INSERT_HEAD(&(b)->list, ap__b, apr_bucket, link); \
+ } while (0)
+
That allows the snippet above to collapse to this:
APR_BRIGADE_INSERT_HEAD(bb, apr_bucket_file_create(fd, 0,
r->finfo.size));
APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_eos_create());
AFAICT, the cost is minimal, and the code is much cleaner looking. Since
these sequences of bucket creation/insertion can be arbitrarily long, this
is a Good Thing IMHO. Thoughts?
On a pseudo-related note, I think I've found some instances of the arguments
to APR_BUCKET_INSERT_foo being swapped (ie, stuff being inserted after an
EOS bucket rather than the other way around) (note that this is completely
understandable because the order of arguments to these macros is backwards
from the arguments to the APR_BRIGADE_INSERT_foo macros, which is bogus)...
but I'll post that as a separate message so that it doesn't get lost in the
cracks and because the particular error I spotted was in Apache, so it
should be posted to new-httpd.
--Cliff
Index: apr_buckets.h
===================================================================
RCS file: /home/cvspublic/apr-util/include/apr_buckets.h,v
retrieving revision 1.73
diff -u -r1.73 apr_buckets.h
--- apr_buckets.h 2001/02/11 15:50:10 1.73
+++ apr_buckets.h 2001/02/13 05:25:30
@@ -316,16 +316,21 @@
* @param e The first bucket in a list of buckets to insert
* @deffunc void APR_BRIGADE_INSERT_HEAD(apr_bucket_brigade *b, apr_bucket
*e)
*/
-#define APR_BRIGADE_INSERT_HEAD(b, e) \
- APR_RING_INSERT_HEAD(&(b)->list, (e), apr_bucket, link)
+#define APR_BRIGADE_INSERT_HEAD(b, e) do { \
+ apr_bucket *ap__b = (e); \
+ APR_RING_INSERT_HEAD(&(b)->list, ap__b, apr_bucket, link); \
+ } while (0)
+
/**
* Insert a list of buckets at the end of a brigade
* @param b The brigade to add to
* @param e The first bucket in a list of buckets to insert
* @deffunc void APR_BRIGADE_INSERT_HEAD(apr_bucket_brigade *b, apr_bucket
*e)
*/
-#define APR_BRIGADE_INSERT_TAIL(b, e) \
- APR_RING_INSERT_TAIL(&(b)->list, (e), apr_bucket, link)
+#define APR_BRIGADE_INSERT_TAIL(b, e) do { \
+ apr_bucket *ap__b = (e); \
+ APR_RING_INSERT_TAIL(&(b)->list, ap__b, apr_bucket, link); \
+ } while (0)
/**
* Concatenate two brigades together
@@ -342,16 +347,21 @@
* @param b The bucket to insert before
* @deffunc void APR_BUCKET_INSERT_BEFORE(apr_bucket *a, apr_bucket *b)
*/
-#define APR_BUCKET_INSERT_BEFORE(a, b) \
- APR_RING_INSERT_BEFORE((a), (b), link)
+#define APR_BUCKET_INSERT_BEFORE(a, b) do { \
+ apr_bucket *ap__a = (a), *ap__b = (b); \
+ APR_RING_INSERT_BEFORE(ap__a, ap__b, link); \
+ } while (0)
+
/**
* Insert a list of buckets after a specified bucket
* @param a The buckets to insert
* @param b The bucket to insert after
* @deffunc void APR_BUCKET_INSERT_AFTER(apr_bucket *a, apr_bucket *b)
*/
-#define APR_BUCKET_INSERT_AFTER(a, b) \
- APR_RING_INSERT_AFTER((a), (b), link)
+#define APR_BUCKET_INSERT_AFTER(a, b) do { \
+ apr_bucket *ap__a = (a), *ap__b = (b); \
+ APR_RING_INSERT_AFTER(ap__a, ap__b, link); \
+ } while (0)
/**
* Get the next bucket in the list
---------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
804-244-8615
Charlottesville, VA
RE: brigade/bucket insertion macros
Posted by Cliff Woolley <cl...@yahoo.com>.
> -----Original Message-----
>> Or do you mean a
>> full patch to mass-cleanup all callers of the macros throughout Apache?
>
> I meant the latter. I can apply what you already sent in about an hour, I
> need to get to the office soon. All that's left is the rest of the
> cleanup. :-)
Ahh. =-) Well in that case, sure, I can do that. It'll take me a little
time though... I'm tied up until about lunchtime EST. But I'll definitely
get this submitted this afternoon.
--Cliff
Re: brigade/bucket insertion macros
Posted by rb...@covalent.net.
> --- rbb@covalent.net wrote:
> > > That allows the snippet above to collapse to this:
> > >
> > > APR_BRIGADE_INSERT_HEAD(bb, apr_bucket_file_create(fd, 0,
> > > r->finfo.size));
> > > APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_eos_create());
> > >
> > > AFAICT, the cost is minimal, and the code is much cleaner looking. Since
> > > these sequences of bucket creation/insertion can be arbitrarily long, this
> > > is a Good Thing IMHO. Thoughts?
> >
> > ++1. If you provide a full patch, I will commit it immediately.
>
> I did, didn't I? It should have been at the bottom of that message. Or do you mean a
> full patch to mass-cleanup all callers of the macros throughout Apache?
I meant the latter. I can apply what you already sent in about an hour, I
need to get to the office soon. All that's left is the rest of the
cleanup. :-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: brigade/bucket insertion macros
Posted by Cliff Woolley <cl...@yahoo.com>.
--- rbb@covalent.net wrote:
> > That allows the snippet above to collapse to this:
> >
> > APR_BRIGADE_INSERT_HEAD(bb, apr_bucket_file_create(fd, 0,
> > r->finfo.size));
> > APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_eos_create());
> >
> > AFAICT, the cost is minimal, and the code is much cleaner looking. Since
> > these sequences of bucket creation/insertion can be arbitrarily long, this
> > is a Good Thing IMHO. Thoughts?
>
> ++1. If you provide a full patch, I will commit it immediately.
I did, didn't I? It should have been at the bottom of that message. Or do you mean a
full patch to mass-cleanup all callers of the macros throughout Apache?
--Cliff
__________________________________________________
Do You Yahoo!?
Get personalized email addresses from Yahoo! Mail - only $35
a year! http://personal.mail.yahoo.com/
Re: brigade/bucket insertion macros
Posted by rb...@covalent.net.
> That allows the snippet above to collapse to this:
>
> APR_BRIGADE_INSERT_HEAD(bb, apr_bucket_file_create(fd, 0,
> r->finfo.size));
> APR_BRIGADE_INSERT_TAIL(bb, apr_bucket_eos_create());
>
> AFAICT, the cost is minimal, and the code is much cleaner looking. Since
> these sequences of bucket creation/insertion can be arbitrarily long, this
> is a Good Thing IMHO. Thoughts?
++1. If you provide a full patch, I will commit it immediately.
Ryan