> -----Original Message----- > Cliff, you just hit on two things that I have spent a lot of time thinking > about, so I'll give my opinions to add to your own.
Glad it's not just me. =-) > I realized that combining them wasn't as easy as you would > think. The first option is to always destroy whenever you call > APR_BUCKET_REMOVE. This doesn't work though, because it means that we > can't move a bucket from one brigade to another > [APR_BUCKET_REMOVE(e);APR_BRIGADE_ADD(...)]. Clearly. And there are probably some other valid reasons to want to do this... Anyway, you're right, this wouldn't work. > The second option is much > more appealing. I would just put a call to APR_BUCKET_REMOVE(e) inside of > apr_bucket_destroy(). This is safe to do, because we can't destroy a > bucket without removing it from the brigade, and if the bucket isn't in > the brigade, the macro is basically a no-op. I thought about that, too. But I was worried that it might be possible to create a bucket and then immediately destroy it, and the ->prev and ->next pointers might be garbage. Upon further inspection, I now see that apr_bucket_do_create uses APR_RING_ELEM_INIT, so this would work. So it's either overloading apr_bucket_destroy() or adding a third macro. I'm game either way. > > (2) I'm ambivalent about the lingering non-capitalization of > > apr_bucket_destroy|read|split|setaside|copy. On the one hand, > > I am against renaming personally, but if you are going to rename, do it > now. :-) Somehow I knew you'd say that. =-) I'm starting to think that if we take care of (1), then (2) won't matter all that much, and we can get by with ignoring it. (Besides, after more thought, I'm swinging back toward thinking that it's more important for apr_bucket_destroy() to match apr_bucket_foo_create() than for it to be capitalized just because it's a macro.) --Cliff
