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 <jw...@virginia.edu> on 2002/02/28 18:57:50 UTC

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

On Wed, 27 Feb 2002, Brian Pane wrote:

> >>  +            if (!APR_BRIGADE_EMPTY(ctx->ssi_tag_brigade)) {
> >>  +                for (;;) {
> >>  +                    apr_bucket *e = APR_BRIGADE_LAST(ctx->ssi_tag_brigade);
> >>  +                    if (e == APR_BRIGADE_SENTINEL(ctx->ssi_tag_brigade)) {
> >>  +                        break;
> >>  +                    }
> >>  +                    APR_BUCKET_REMOVE(e);
> >>  +                    APR_BRIGADE_INSERT_HEAD(bb, e);
> >>  +                }
> >>               }
> >>
>
> I was thinking about APR_BRIGADE_CONCAT, but what I really needed
> was APR_BRIGADE_PREPEND because _CONCAT would have left bb empty.
> Is there a "prepend" variant anywhere?

The following is equivalent to the above (except it runs in constant
time):

/* prepend ctx->ssi_tag_brigade onto bb */
APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);

But I'd be fine with adding a prepend macro in the API if others agree
it's useful.

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



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

Posted by Brian Pane <br...@cnet.com>.
Cliff Woolley wrote:

>On Wed, 27 Feb 2002, Brian Pane wrote:
>
>>>> +            if (!APR_BRIGADE_EMPTY(ctx->ssi_tag_brigade)) {
>>>> +                for (;;) {
>>>> +                    apr_bucket *e = APR_BRIGADE_LAST(ctx->ssi_tag_brigade);
>>>> +                    if (e == APR_BRIGADE_SENTINEL(ctx->ssi_tag_brigade)) {
>>>> +                        break;
>>>> +                    }
>>>> +                    APR_BUCKET_REMOVE(e);
>>>> +                    APR_BRIGADE_INSERT_HEAD(bb, e);
>>>> +                }
>>>>              }
>>>>
>>I was thinking about APR_BRIGADE_CONCAT, but what I really needed
>>was APR_BRIGADE_PREPEND because _CONCAT would have left bb empty.
>>Is there a "prepend" variant anywhere?
>>
>
>The following is equivalent to the above (except it runs in constant
>time):
>
>/* prepend ctx->ssi_tag_brigade onto bb */
>APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
>APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
>

Thanks, I'll update this in mod_include later today.

>But I'd be fine with adding a prepend macro in the API if others agree
>it's useful.
>

+1

--Brian




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

Posted by Brian Pane <br...@cnet.com>.
Cliff Woolley wrote:

>On Wed, 27 Feb 2002, Brian Pane wrote:
>
>>>> +            if (!APR_BRIGADE_EMPTY(ctx->ssi_tag_brigade)) {
>>>> +                for (;;) {
>>>> +                    apr_bucket *e = APR_BRIGADE_LAST(ctx->ssi_tag_brigade);
>>>> +                    if (e == APR_BRIGADE_SENTINEL(ctx->ssi_tag_brigade)) {
>>>> +                        break;
>>>> +                    }
>>>> +                    APR_BUCKET_REMOVE(e);
>>>> +                    APR_BRIGADE_INSERT_HEAD(bb, e);
>>>> +                }
>>>>              }
>>>>
>>I was thinking about APR_BRIGADE_CONCAT, but what I really needed
>>was APR_BRIGADE_PREPEND because _CONCAT would have left bb empty.
>>Is there a "prepend" variant anywhere?
>>
>
>The following is equivalent to the above (except it runs in constant
>time):
>
>/* prepend ctx->ssi_tag_brigade onto bb */
>APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
>APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
>

Thanks, I'll update this in mod_include later today.

>But I'd be fine with adding a prepend macro in the API if others agree
>it's useful.
>

+1

--Brian




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

Posted by Brian Pane <bp...@pacbell.net>.
Cliff Woolley wrote:

>On Thu, 28 Feb 2002, Cliff Woolley wrote:
>
>>On Thu, 28 Feb 2002, Greg Stein wrote:
>>
>>>>/* prepend ctx->ssi_tag_brigade onto bb */
>>>>APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
>>>>APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
>>>>
>>>That is *very* obtuse. I can't imagine anybody figuring out that idiom
>>>means "prepend". Yes, please add an appropriate macro.
>>>
>>Okay, will add.
>>
>
>Committed.  Brian, can you please run your tests on it for me?  I've yet
>to find the exact test case to get into that block of code.
>

I just finished testing against my test cases that exercise this
part of the code, and the new version using APR_BRIGADE_PREPEND
works fine.

All my test cases for this are variants of a technique that Ian
created: use mod_bucketeer in front of mod_include, and insert
ctrl-B's (force new bucket), ctrl-P's (pass brigade now), and
ctrl-F's (flush brigade) after '<' symbols that aren't the start
of a "<!--#".

This exercises the code in mod_include that keeps track of
<!--# directives that span brigades.

--Brian



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

Posted by Brian Pane <bp...@pacbell.net>.
Cliff Woolley wrote:

>On Thu, 28 Feb 2002, Cliff Woolley wrote:
>
>>On Thu, 28 Feb 2002, Greg Stein wrote:
>>
>>>>/* prepend ctx->ssi_tag_brigade onto bb */
>>>>APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
>>>>APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
>>>>
>>>That is *very* obtuse. I can't imagine anybody figuring out that idiom
>>>means "prepend". Yes, please add an appropriate macro.
>>>
>>Okay, will add.
>>
>
>Committed.  Brian, can you please run your tests on it for me?  I've yet
>to find the exact test case to get into that block of code.
>

I just finished testing against my test cases that exercise this
part of the code, and the new version using APR_BRIGADE_PREPEND
works fine.

All my test cases for this are variants of a technique that Ian
created: use mod_bucketeer in front of mod_include, and insert
ctrl-B's (force new bucket), ctrl-P's (pass brigade now), and
ctrl-F's (flush brigade) after '<' symbols that aren't the start
of a "<!--#".

This exercises the code in mod_include that keeps track of
<!--# directives that span brigades.

--Brian



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

Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 28 Feb 2002, Cliff Woolley wrote:

> On Thu, 28 Feb 2002, Greg Stein wrote:
>
> > > /* prepend ctx->ssi_tag_brigade onto bb */
> > > APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
> > > APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
> >
> > That is *very* obtuse. I can't imagine anybody figuring out that idiom
> > means "prepend". Yes, please add an appropriate macro.
>
> Okay, will add.

Committed.  Brian, can you please run your tests on it for me?  I've yet
to find the exact test case to get into that block of code.

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



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

Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 28 Feb 2002, Cliff Woolley wrote:

> On Thu, 28 Feb 2002, Greg Stein wrote:
>
> > > /* prepend ctx->ssi_tag_brigade onto bb */
> > > APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
> > > APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
> >
> > That is *very* obtuse. I can't imagine anybody figuring out that idiom
> > means "prepend". Yes, please add an appropriate macro.
>
> Okay, will add.

Committed.  Brian, can you please run your tests on it for me?  I've yet
to find the exact test case to get into that block of code.

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



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

Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 28 Feb 2002, Greg Stein wrote:

> > /* prepend ctx->ssi_tag_brigade onto bb */
> > APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
> > APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
>
> That is *very* obtuse. I can't imagine anybody figuring out that idiom means
> "prepend". Yes, please add an appropriate macro.

Okay, will add.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



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

Posted by Cliff Woolley <jw...@virginia.edu>.
On Thu, 28 Feb 2002, Greg Stein wrote:

> > /* prepend ctx->ssi_tag_brigade onto bb */
> > APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
> > APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
>
> That is *very* obtuse. I can't imagine anybody figuring out that idiom means
> "prepend". Yes, please add an appropriate macro.

Okay, will add.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



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

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 28, 2002 at 12:57:50PM -0500, Cliff Woolley wrote:
>...
> The following is equivalent to the above (except it runs in constant
> time):
> 
> /* prepend ctx->ssi_tag_brigade onto bb */
> APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
> APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
> 
> But I'd be fine with adding a prepend macro in the API if others agree
> it's useful.

That is *very* obtuse. I can't imagine anybody figuring out that idiom means
"prepend". Yes, please add an appropriate macro.

thx!
-g

-- 
Greg Stein, http://www.lyra.org/

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

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 28, 2002 at 12:57:50PM -0500, Cliff Woolley wrote:
>...
> The following is equivalent to the above (except it runs in constant
> time):
> 
> /* prepend ctx->ssi_tag_brigade onto bb */
> APR_BRIGADE_CONCAT(ctx->ssi_tag_brigade, bb);
> APR_BRIGADE_CONCAT(bb, ctx->ssi_tag_brigade);
> 
> But I'd be fine with adding a prepend macro in the API if others agree
> it's useful.

That is *very* obtuse. I can't imagine anybody figuring out that idiom means
"prepend". Yes, please add an appropriate macro.

thx!
-g

-- 
Greg Stein, http://www.lyra.org/