apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: [PATCH] brigade buffering.
Date Thu, 25 Jan 2001 21:51:02 GMT

> 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.

I need to think about these, but I think you are correct.  I am not
proposing that this is bullet-proof code, just that it is an idea that
should be considered.

> 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);

This is not an issue, since it will never happen.  Remember, that I said
up front that these functions are not designed to be called by
programmers.  These are designed to be called by Apache's internal
functions.  In this case, the programmer has called either ap_f* or ap_r*,
which passes things down the brigade.

However, you are correct that in general this is an issue, the solution is
to always copy the data, but that has other drawbacks.  So, there are two
answers to this that I see.  #1, tell the programmer that (s)he is out of
luck if (s)he doesn't understand the code.  #2, actually copy all the
data.

I have no problem telling the programmer they are out of luck, this is
open source after all.  I dislike copying the data, because it means we
could chew a lot of memory very quickly.

Ryan
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Mime
View raw message