On Thu, Apr 2, 2020 at 11:28 AM Ruediger Pluem <rpl...@apache.org> wrote:
>
> So how about
>
> #define AP_BUCKET_HAS_UNKNOWN_LENGTH(e) ((e)->length == (apr_size_t)-1)
Still quite mouthful..
What about? :
/**
* Determine if a bucket is opaque, i.e. of unknown data length and type
* @param e The bucket to inspect
* @return true or false
*/
#define AP_BUCKET_IS_OPAQUE(e) ((e)->length == (apr_size_t)-1)
Unlike FILE buckets with struct apr_bucket_file where we can get the
->fd, PIPE and SOCKET buckets don't expose their data type in the API,
relying on (apr_{file,socket}_t *)e->data is not really API/ABI safe
(though APR is unlikely to change that). I mean, while buckets are
generally quite opaque and should be manipulated with the API methods,
these "->length == -1" once are particularly opaque :)
>
> It should be in APR, but that takes quite long until it arrives in httpd. So
> we might create one here and in APR in parallel
> and define the AP_ conditionally as the APR_ one if it exists and otherwise
> on our own.
+1
Regards,
Yann.
Index: include/http_request.h
===================================================================
--- include/http_request.h (revision 1876034)
+++ include/http_request.h (working copy)
@@ -589,13 +589,11 @@ AP_DECLARE(int) ap_if_walk(request_rec *r);
AP_DECLARE_DATA extern const apr_bucket_type_t ap_bucket_type_eor;
/**
- * Determine if a bucket is morphing, that is which changes its
- * type on read (usually to "heap" allocated data), while moving
- * itself at the next position to remain plugged until exhausted.
+ * Determine if a bucket is opaque, i.e. of unknown data type and length
* @param e The bucket to inspect
* @return true or false
*/
-#define AP_BUCKET_IS_MORPHING(e) ((e)->length == (apr_size_t)-1)
+#define AP_BUCKET_IS_OPAQUE(e) ((e)->length == (apr_size_t)-1)
/**
* Determine if a bucket is an End Of REQUEST (EOR) bucket
Index: modules/cache/mod_cache.c
===================================================================
--- modules/cache/mod_cache.c (revision 1876034)
+++ modules/cache/mod_cache.c (working copy)
@@ -1281,7 +1281,7 @@ static apr_status_t cache_save_filter(ap_filter_t
if (APR_BUCKET_IS_FLUSH(e)) {
continue;
}
- if (e->length == (apr_size_t)-1) {
+ if (AP_BUCKET_IS_OPAQUE(e)) {
break;
}
size += e->length;
Index: modules/http/byterange_filter.c
===================================================================
--- modules/http/byterange_filter.c (revision 1876034)
+++ modules/http/byterange_filter.c (working copy)
@@ -435,7 +435,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_byterange_
*/
for (e = APR_BRIGADE_FIRST(bb);
(e != APR_BRIGADE_SENTINEL(bb) && !APR_BUCKET_IS_EOS(e)
- && e->length != (apr_size_t)-1);
+ && !AP_BUCKET_IS_OPAQUE(e));
e = APR_BUCKET_NEXT(e)) {
clength += e->length;
}
Index: modules/http/chunk_filter.c
===================================================================
--- modules/http/chunk_filter.c (revision 1876034)
+++ modules/http/chunk_filter.c (working copy)
@@ -88,7 +88,7 @@ apr_status_t ap_http_chunk_filter(ap_filter_t *f,
}
break;
}
- else if (e->length == (apr_size_t)-1) {
+ else if (AP_BUCKET_IS_OPAQUE(e)) {
/* unknown amount of data (e.g. a pipe) */
const char *data;
apr_size_t len;
Index: modules/http2/h2_stream.c
===================================================================
--- modules/http2/h2_stream.c (revision 1876034)
+++ modules/http2/h2_stream.c (working copy)
@@ -21,6 +21,7 @@
#include <httpd.h>
#include <http_core.h>
+#include <http_request.h>
#include <http_connection.h>
#include <http_log.h>
@@ -866,7 +867,7 @@ static apr_status_t add_buffered_data(h2_stream *s
apr_bucket_destroy(b);
}
else {
- ap_assert(b->length != (apr_size_t)-1);
+ ap_assert(!AP_BUCKET_IS_OPAQUE(b));
*plen += b->length;
if (*plen >= requested) {
*plen = requested;
Index: server/core_filters.c
===================================================================
--- server/core_filters.c (revision 1876034)
+++ server/core_filters.c (working copy)
@@ -277,7 +277,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f,
while ((len < readbytes) && (rv == APR_SUCCESS)
&& (e != APR_BRIGADE_SENTINEL(ctx->bb))) {
/* Check for the availability of buckets with known length */
- if (e->length != (apr_size_t)-1) {
+ if (!AP_BUCKET_IS_OPAQUE(e)) {
len += e->length;
e = APR_BUCKET_NEXT(e);
}
Index: server/protocol.c
===================================================================
--- server/protocol.c (revision 1876034)
+++ server/protocol.c (working copy)
@@ -1844,7 +1844,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_content_le
}
/* For determinate length data buckets, count the length and
* continue. */
- else if (e->length != (apr_size_t)-1) {
+ else if (!AP_BUCKET_IS_OPAQUE(e)) {
r->bytes_sent += e->length;
e = APR_BUCKET_NEXT(e);
continue;
Index: server/util_filter.c
===================================================================
--- server/util_filter.c (revision 1876034)
+++ server/util_filter.c (working copy)
@@ -976,10 +976,10 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigad
e = next) {
next = APR_BUCKET_NEXT(e);
- /* Morphing buckets are moved, so assumed to have next EOR's
+ /* Opaque buckets are moved, so assumed to have next EOR's
* lifetime or at least the lifetime of the connection.
*/
- if (AP_BUCKET_IS_MORPHING(e)) {
+ if (AP_BUCKET_IS_OPAQUE(e)) {
/* Save buckets batched below? */
if (batched_buckets) {
batched_buckets = 0;
@@ -1058,7 +1058,7 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
{
apr_bucket *bucket, *next;
apr_size_t bytes_in_brigade, memory_bytes_in_brigade;
- int eor_buckets_in_brigade, morphing_buckets_in_brigade;
+ int eor_buckets_in_brigade, opaque_buckets_in_brigade;
struct ap_filter_private *fp = f->priv;
core_server_config *conf;
@@ -1094,7 +1094,7 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
* of everything up to the last one.
*
* b) The brigade contains at least flush_max_threshold bytes in memory,
- * that is non-file and non-morphing buckets: do blocking writes of
+ * that is non-file and non-opaque buckets: do blocking writes of
* everything up the last bucket above flush_max_threshold.
* (The point of this rule is to provide flow control, in case a
* handler is streaming out lots of data faster than the data can be
@@ -1105,9 +1105,9 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
* (The point of this rule is to prevent too many FDs being kept open
* by pipelined requests, possibly allowing a DoS).
*
- * Note that morphing buckets use no memory until read, so they don't
+ * Note that opaque buckets use no memory until read, so they don't
* account for point b) above. Both ap_filter_reinstate_brigade() and
- * ap_filter_setaside_brigade() assume that morphing buckets have an
+ * ap_filter_setaside_brigade() assume that opaque buckets have an
* appropriate lifetime (until next EOR for instance), so they are simply
* setaside or reinstated by moving them from/to fp->bb to/from bb.
*/
@@ -1115,7 +1115,7 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
bytes_in_brigade = 0;
memory_bytes_in_brigade = 0;
eor_buckets_in_brigade = 0;
- morphing_buckets_in_brigade = 0;
+ opaque_buckets_in_brigade = 0;
conf = ap_get_core_module_config(f->c->base_server->module_config);
@@ -1126,8 +1126,8 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
if (AP_BUCKET_IS_EOR(bucket)) {
eor_buckets_in_brigade++;
}
- else if (AP_BUCKET_IS_MORPHING(bucket)) {
- morphing_buckets_in_brigade++;
+ else if (AP_BUCKET_IS_OPAQUE(bucket)) {
+ opaque_buckets_in_brigade++;
}
else if (bucket->length) {
bytes_in_brigade += bucket->length;
@@ -1151,13 +1151,13 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, f->c,
"seen in brigade%s: bytes: %" APR_SIZE_T_FMT
", memory bytes: %" APR_SIZE_T_FMT ", eor "
- "buckets: %d, morphing buckets: %d",
+ "buckets: %d, opaque buckets: %d",
*flush_upto == NULL ? " so far"
: " since last flush point",
bytes_in_brigade,
memory_bytes_in_brigade,
eor_buckets_in_brigade,
- morphing_buckets_in_brigade);
+ opaque_buckets_in_brigade);
}
/*
* Defer the actual blocking write to avoid doing many writes.
@@ -1167,7 +1167,7 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
bytes_in_brigade = 0;
memory_bytes_in_brigade = 0;
eor_buckets_in_brigade = 0;
- morphing_buckets_in_brigade = 0;
+ opaque_buckets_in_brigade = 0;
}
}
@@ -1174,10 +1174,10 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_briga
ap_log_cerror(APLOG_MARK, APLOG_TRACE8, 0, f->c,
"brigade contains%s: bytes: %" APR_SIZE_T_FMT
", non-file bytes: %" APR_SIZE_T_FMT
- ", eor buckets: %d, morphing buckets: %d",
+ ", eor buckets: %d, opaque buckets: %d",
*flush_upto == NULL ? "" : " since last flush point",
bytes_in_brigade, memory_bytes_in_brigade,
- eor_buckets_in_brigade, morphing_buckets_in_brigade);
+ eor_buckets_in_brigade, opaque_buckets_in_brigade);
return APR_SUCCESS;
}
@@ -1308,7 +1308,7 @@ AP_DECLARE_NONSTD(int) ap_filter_input_pending(con
fp = APR_RING_PREV(fp, pending)) {
apr_bucket *e;
- /* if there is a leading non-morphing bucket
+ /* if there is a leading non-opaque bucket
* in place, then we have data pending
*/
AP_DEBUG_ASSERT(fp->bb);
