On Thu, Sep 01, 2011 at 02:47:19PM +0200, "Plüm, Rüdiger, VF-Group" wrote:
> > > If we rip it out, we should replace it with ap_assert()s. And maybe
> > > only do it in 2.3?
> >
> > It would seem odd to have ENOTIMPL as a "fatal" error but other
> > "real" errors non-fatal. *No* error should occur here with unless
> > somebody has cooked up a really whacky bucket type.
>
> So you would follow the rip-out-and-replace-with-asserts approach?
No, just remove the special case for ENOTIMPL and don't change anything
else.
Index: modules/http/byterange_filter.c
===================================================================
--- modules/http/byterange_filter.c (revision 1163995)
+++ modules/http/byterange_filter.c (working copy)
@@ -94,8 +94,6 @@
apr_bucket *first = NULL, *last = NULL, *out_first = NULL, *e;
apr_uint64_t pos = 0, off_first = 0, off_last = 0;
apr_status_t rv;
- const char *s;
- apr_size_t len;
apr_uint64_t start64, end64;
apr_off_t pofft = 0;
@@ -147,44 +145,10 @@
if (e == first) {
if (off_first != start64) {
rv = apr_bucket_split(copy, (apr_size_t)(start64 - off_first));
- if (rv == APR_ENOTIMPL) {
- rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
- if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
- /*
- * The read above might have morphed copy in a bucket
- * of shorter length. So read and delete until we reached
- * the correct bucket for splitting.
- */
- while (start64 - off_first > (apr_uint64_t)copy->length) {
- apr_bucket *tmp = APR_BUCKET_NEXT(copy);
- off_first += (apr_uint64_t)copy->length;
- APR_BUCKET_REMOVE(copy);
- apr_bucket_destroy(copy);
- copy = tmp;
- rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
- if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
- }
- if (start64 > off_first) {
- rv = apr_bucket_split(copy, (apr_size_t)(start64 -
off_first));
- if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
- }
- else {
- copy = APR_BUCKET_PREV(copy);
- }
+ if (rv != APR_SUCCESS) {
+ apr_brigade_cleanup(bbout);
+ return rv;
}
- else if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
out_first = APR_BUCKET_NEXT(copy);
APR_BUCKET_REMOVE(copy);
apr_bucket_destroy(copy);
@@ -200,38 +164,10 @@
}
if (end64 - off_last != (apr_uint64_t)e->length) {
rv = apr_bucket_split(copy, (apr_size_t)(end64 + 1 -
off_last));
- if (rv == APR_ENOTIMPL) {
- rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
- if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
- /*
- * The read above might have morphed copy in a bucket
- * of shorter length. So read until we reached
- * the correct bucket for splitting.
- */
- while (end64 + 1 - off_last > (apr_uint64_t)copy->length) {
- off_last += (apr_uint64_t)copy->length;
- copy = APR_BUCKET_NEXT(copy);
- rv = apr_bucket_read(copy, &s, &len, APR_BLOCK_READ);
- if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
- }
- if (end64 < off_last + (apr_uint64_t)copy->length - 1) {
- rv = apr_bucket_split(copy, (apr_size_t)(end64 + 1 -
off_last));
- if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
- }
+ if (rv != APR_SUCCESS) {
+ apr_brigade_cleanup(bbout);
+ return rv;
}
- else if (rv != APR_SUCCESS) {
- apr_brigade_cleanup(bbout);
- return rv;
- }
copy = APR_BUCKET_NEXT(copy);
if (copy != APR_BRIGADE_SENTINEL(bbout)) {
APR_BUCKET_REMOVE(copy);
