Author: kotkov
Date: Mon Dec 22 19:30:13 2025
New Revision: 1930807
Log:
ra_serf: Implement and start using a default readline handler for
custom bucket types.
Before this change, we were making an assumption that the calling site
is not going to call bucket->readline() on our custom bucket types.
This is not something a bucket type has control over, and the assumption
may not hold in the future.
To solve this issue, we provide a default bucket readline implementation,
modeled over the current implementation of serf_default_readline() in
serf trunk.
* subversion/libsvn_ra_serf/ra_serf.h
(svn_ra_serf__default_readline): New function.
* subversion/libsvn_ra_serf/util.c
(svn_ra_serf__default_readline): Implement based on the current
implementation of serf_default_readline() in serf trunk.
* subversion/libsvn_ra_serf/eagain_bucket.c
* subversion/libsvn_ra_serf/sb_bucket.c:
Use svn_ra_serf__default_readline() as the readline handler.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c
subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c
subversion/trunk/subversion/libsvn_ra_serf/util.c
Modified: subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c Mon Dec 22 18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/eagain_bucket.c Mon Dec 22 19:30:13 2025 (r1930807)
@@ -66,21 +66,6 @@ eagain_bucket_read(serf_bucket_t *bucket
return APR_EAGAIN;
}
-#if !SERF_VERSION_AT_LEAST(1, 4, 0)
-static apr_status_t
-eagain_bucket_readline(serf_bucket_t *bucket,
- int acceptable,
- int *found,
- const char **data,
- apr_size_t *len)
-{
- /* ### for now, we know callers won't use this function. */
- svn_error_clear(svn_error__malfunction(TRUE, __FILE__, __LINE__,
- "Not implemented."));
- return APR_ENOTIMPL;
-}
-#endif
-
static apr_status_t
eagain_bucket_peek(serf_bucket_t *bucket,
@@ -99,11 +84,7 @@ eagain_bucket_peek(serf_bucket_t *bucket
static const serf_bucket_type_t delay_bucket_vtable = {
"BUF-EAGAIN",
eagain_bucket_read,
-#if SERF_VERSION_AT_LEAST(1, 4, 0)
- serf_default_readline,
-#else
- eagain_bucket_readline,
-#endif
+ svn_ra_serf__default_readline,
serf_default_read_iovec,
serf_default_read_for_sendfile,
serf_default_read_bucket,
Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Mon Dec 22 18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Mon Dec 22 19:30:13 2025 (r1930807)
@@ -1667,6 +1667,15 @@ svn_ra_serf__create_stream_bucket(svn_st
svn_ra_serf__stream_bucket_errfunc_t errfunc,
void *errfunc_baton);
+/* Default implementation of the bucket readline function, see
+ serf_bucket_type_t.readline. This function will use the bucket
+ read function, when possible optimized by the bucket peek function
+ to return the requested result. */
+apr_status_t
+svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
+ int *found,
+ const char **data, apr_size_t *len);
+
#if defined(SVN_DEBUG)
/* Wrapper macros to collect file and line information */
#define svn_ra_serf__wrap_err \
Modified: subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c Mon Dec 22 18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/sb_bucket.c Mon Dec 22 19:30:13 2025 (r1930807)
@@ -117,19 +117,6 @@ sb_bucket_read(serf_bucket_t *bucket, ap
return *data == NULL ? APR_EOF : APR_SUCCESS;
}
-#if !SERF_VERSION_AT_LEAST(1, 4, 0)
-static apr_status_t
-sb_bucket_readline(serf_bucket_t *bucket, int acceptable,
- int *found,
- const char **data, apr_size_t *len)
-{
- /* ### for now, we know callers won't use this function. */
- svn_error_clear(svn_error__malfunction(TRUE, __FILE__, __LINE__,
- "Not implemented."));
- return APR_ENOTIMPL;
-}
-#endif
-
static apr_status_t
sb_bucket_peek(serf_bucket_t *bucket,
@@ -160,11 +147,7 @@ sb_bucket_peek(serf_bucket_t *bucket,
static const serf_bucket_type_t sb_bucket_vtable = {
"SPILLBUF",
sb_bucket_read,
-#if SERF_VERSION_AT_LEAST(1, 4, 0)
- serf_default_readline,
-#else
- sb_bucket_readline,
-#endif
+ svn_ra_serf__default_readline,
serf_default_read_iovec,
serf_default_read_for_sendfile,
serf_default_read_bucket,
Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c Mon Dec 22 18:45:03 2025 (r1930806)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Mon Dec 22 19:30:13 2025 (r1930807)
@@ -39,6 +39,7 @@
#include "svn_string.h"
#include "svn_props.h"
#include "svn_dirent_uri.h"
+#include "svn_sorts.h"
#include "../libsvn_ra/ra_loader.h"
#include "private/svn_dep_compat.h"
@@ -2154,3 +2155,128 @@ svn_ra_serf__get_dirent_props(apr_uint32
return props;
}
+static apr_status_t
+bucket_limited_readline(serf_bucket_t *bucket, int acceptable,
+ apr_size_t requested, int *found,
+ const char **data, apr_size_t *len)
+{
+ apr_status_t status;
+ const char *peek_data;
+ apr_size_t peek_len;
+
+ status = bucket->type->peek(bucket, &peek_data, &peek_len);
+ if (SERF_BUCKET_READ_ERROR(status))
+ return status;
+
+ if (peek_len == 0)
+ {
+ /* peek() returned no data. */
+
+ /* ... if that's because the bucket has no data, then we're done. */
+ if (APR_STATUS_IS_EOF(status))
+ {
+ *found = SERF_NEWLINE_NONE;
+ *len = 0;
+ return APR_EOF;
+ }
+
+ /* We can only read and return a single character.
+
+ For example, if we tried reading 2 characters seeking CRLF, and
+ got CR followed by 'a', then we have over-read the line, and
+ consumed a character from the next line. Bad.
+
+ The only exception is when we *only* allow CRLF as newline. In that
+ case CR followed by 'a' would just be raw line data, not a line
+ break followed by data. If we allow any other type of newline we
+ can't use this trick.
+ */
+
+ if ((acceptable & SERF_NEWLINE_ANY) == SERF_NEWLINE_CRLF)
+ requested = MIN(requested, 2); /* Only CRLF is allowed */
+ else
+ requested = MIN(requested, 1);
+ }
+ else
+ {
+ /* peek_len > 0 */
+
+ const char *cr = NULL;
+ const char *lf = NULL;
+
+ if (peek_len > requested)
+ peek_len = requested;
+
+ if ((acceptable & SERF_NEWLINE_CR) || (acceptable & SERF_NEWLINE_CRLF))
+ cr = memchr(peek_data, '\r', peek_len);
+ if ((acceptable & SERF_NEWLINE_LF))
+ lf = memchr(peek_data, '\n', peek_len);
+
+ if (cr && lf)
+ cr = MIN(cr, lf);
+ else if (lf)
+ cr = lf;
+
+ /* ### When we are only looking for CRLF we may return too small
+ chunks here when the data contains CR or LF without the other.
+ That isn't incorrect, but it could be optimized.
+
+ ### But as that case is not common, the caller has to assume
+ partial reads anyway and this is just a not very inefficient
+ fallback implementation...
+
+ Let's make the buffering in the caller handle that case
+ for now. */
+
+ if (cr && *cr == '\r' && (acceptable & SERF_NEWLINE_CRLF) &&
+ ((cr + 1) < (peek_data + peek_len)) && *(cr + 1) == '\n')
+ {
+ requested = (cr + 2) - peek_data;
+ }
+ else if (cr)
+ requested = (cr + 1) - peek_data;
+ else
+ requested = peek_len;
+ }
+
+ status = bucket->type->read(bucket, requested, data, len);
+ if (SERF_BUCKET_READ_ERROR(status))
+ return status;
+
+ if (*len == 0)
+ {
+ *found = SERF_NEWLINE_NONE;
+ }
+ else if ((acceptable & SERF_NEWLINE_CRLF) && *len >= 2 &&
+ (*data)[*len - 1] == '\n' && (*data)[*len - 2] == '\r')
+ {
+ *found = SERF_NEWLINE_CRLF;
+ }
+ else if ((acceptable & SERF_NEWLINE_LF) && (*data)[*len - 1] == '\n')
+ {
+ *found = SERF_NEWLINE_LF;
+ }
+ else if ((acceptable & (SERF_NEWLINE_CRLF | SERF_NEWLINE_CR)) &&
+ (*data)[*len - 1] == '\r')
+ {
+ *found = (acceptable & (SERF_NEWLINE_CRLF)) ? SERF_NEWLINE_CRLF_SPLIT
+ : SERF_NEWLINE_CR;
+ }
+ else
+ *found = SERF_NEWLINE_NONE;
+
+ return status;
+}
+
+apr_status_t
+svn_ra_serf__default_readline(serf_bucket_t *bucket, int acceptable,
+ int *found,
+ const char **data, apr_size_t *len)
+{
+#if SERF_VERSION_AT_LEAST(1, 4, 0)
+ return serf_default_readline(bucket, acceptable, found, data, len);
+#else
+ return bucket_limited_readline(bucket, acceptable, SERF_READ_ALL_AVAIL,
+ found, data, len);
+#endif
+}
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.