Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 59357 invoked by uid 500); 13 Feb 2001 06:13:27 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 59339 invoked from network); 13 Feb 2001 06:13:26 -0000 From: Cliff Woolley To: dev@apr.apache.org MMDF-Warning: Parse error in original version of preceding line at mail.virginia.edu Subject: brigade/bucket insertion macros Date: Tue, 13 Feb 2001 01:13:05 -0500 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Priority: 3 (Normal) X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook IMO, Build 9.0.2416 (9.0.2911.0) Importance: Normal X-MimeOLE: Produced By Microsoft MimeOLE V5.00.2615.200 X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N 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