From gregm@alum.wpi.edu  Thu Jan 25 21:29:48 2001
Return-Path: <gregm@alum.wpi.edu>
Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Delivered-To: mailing list dev@apr.apache.org
Received: (qmail 83992 invoked from network); 25 Jan 2001 21:29:48 -0000
Received: from antiochus-fe0.ultra.net (146.115.8.188)
  by h31.sny.collab.net with SMTP; 25 Jan 2001 21:29:48 -0000
Received: from kosh.alum.wpi.edu ([209.6.19.90]) by antiochus-fe0.ultra.net (8.8.8/ult/n20340/mtc.v2) with ESMTP id QAA06027 for <dev@apr.apache.org>; Thu, 25 Jan 2001 16:28:25 -0500 (EST)
Message-Id: <5.0.2.1.2.20010125155304.00a868d0@pop.charter.net>
X-Sender: gregmm@pop.charter.net
X-Mailer: QUALCOMM Windows Eudora Version 5.0.2
Date: Thu, 25 Jan 2001 16:27:25 -0500
To: dev@apr.apache.org
From: Greg Marr <gregm@alum.wpi.edu>
Subject: Re: [PATCH] brigade buffering.
In-Reply-To: <Pine.LNX.4.21.0101251238340.4928-100000@koj>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N

At 03:41 PM 01/25/2001, rbb@covalent.net wrote:
>This general patch has been on new-httpd, but it really belongs 
>here.  This is my general concept for how brigades should be buffered.

FYI: this has the potential to fail miserably:

     char buffer[APR_BUCKET_BUFF_SIZE + 1];
     int i;

     for(i = 0; i < APR_BUCKET_BUFF_SIZE + 1; ++i) {
         buffer[i] = 'a' + (i % 26);
     }

     apr_brigade_write(b, buffer, 1);
     apr_brigade_write(b, buffer + 1, APR_BUCKET_BUFF_SIZE);

In the second call, check_brigade_flush returns 0, with nbyte set to 
1, so 1 byte is copied from str, except that it's the wrong byte, 
since str is the same as it was upon entering. (hence the
buffer[i] = 'a' + (i % 26);
initialization, you wouldn't see this with a buffer of all a's.)

A similar failure occurs at APR_BUCKET_BUFF_SIZE * 2 + 1, where a 
transient bucket will be created containing the first 
APR_BUCKET_BUFF_SIZE + 1 bytes of str.  These both can be fixed by 
changing str to const char ** in check_brigade_flush, and moving the 
pointer as you go.

A third failure is that check_brigade_flush creates a transient 
bucket and sticks it in the brigade.  Here's where this can fail:

     char buf[APR_BUCKET_BUFF_SIZE * 2 + 1];

     apr_vsnprintf(buf, APR_BUCKET_BUFF_SIZE * 2 + 1, fmt, va);

     return apr_brigade_puts(b, buf);

b now can contain a transient bucket pointing to stack space that has 
been reclaimed.

or also here:

     char buffer[APR_BUCKET_BUFF_SIZE * 2 + 1];
     int i;

     for(i = 0; i < APR_BUCKET_BUFF_SIZE * 2 + 1; ++i) {
         buffer[i] = 'a' + (i % 26);
     }

     apr_brigade_write(b, buffer, 1);
     apr_brigade_write(b, buffer + 1, APR_BUCKET_BUFF_SIZE * 2);

     for(i = 0; i < APR_BUCKET_BUFF_SIZE * 2 + 1; ++i) {
         buffer[i] = '0' + (i % 10);
     }

     apr_brigade_write(b, buffer, APR_BUCKET_BUFF_SIZE * 2 + 1);

-- 
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"