apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <cliffwool...@yahoo.com>
Subject brigade/bucket insertion macros
Date Tue, 13 Feb 2001 06:13:05 GMT

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


Mime
View raw message