serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@gmail.com>
Subject Re: svn commit: r1713936 - in /serf/trunk: buckets/allocator.c buckets/log_wrapper_buckets.c outgoing.c test/mock_buckets.c
Date Thu, 12 Nov 2015 00:44:00 GMT
On Wed, Nov 11, 2015 at 3:29 PM, <rhuijben@apache.org> wrote:
>...

> +++ serf/trunk/buckets/allocator.c Wed Nov 11 21:29:45 2015
> @@ -416,9 +416,11 @@ apr_status_t serf_debug__record_read(
>      read_status_t *rs = find_read_status(track, bucket, 1);
>
>      /* Validate that the previous status value allowed for another read.
> */
> -    if (APR_STATUS_IS_EAGAIN(rs->last) /* ### or APR_EOF? */) {
> +    if (SERF_BUCKET_READ_ERROR(rs->last)) {
>

This is wrong. If a bucket returns APR_EAGAIN, you are *not* supposed to
read the bucket again until the context loop has been invoked.

I agree that you should stop reading if a READ_ERROR occurs. That is a good
addition. But that excludes EAGAIN, incorrectly.

>...

> @@ -492,6 +497,27 @@ void serf_debug__bucket_destroy(const se
>          if (SERF_BUCKET_IS_BARRIER(bucket))
>              return;
>
> +        if (SERF_BUCKET_IS_AGGREGATE(bucket)) {
> +            apr_status_t status;
> +            const char *data;
> +            apr_size_t len;
> +
> +            serf_bucket_aggregate_hold_open(bucket, NULL, NULL);
> +
> +            status = serf_bucket_read(bucket, SERF_READ_ALL_AVAIL,
> +                                      &data, &len);
> +
> +            if (APR_STATUS_IS_EOF(status) && !len)
> +                return;
> +        }
> +
> +        bkt = serf_bucket_read_bucket((serf_bucket_t*)bucket,
> +                                      &serf_bucket_type_ssl_encrypt);
> +
> +        if (bkt != NULL) {
> +            serf_bucket_destroy(bkt);
> +            return;
> +        }
>

What is all that?? How about some comments, please.

>...

Cheers,
-g

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message