[This mail is mostly to Greg Stein, but Greg Hudson, search for
"Hudson" below for an issue that may be of interest to you. -karl]
gstein@tigris.org writes:
> --- OLD/trunk/subversion/mod_dav_svn/repos.c Mon Jan 14 19:09:45 2002
> +++ NEW/trunk/subversion/mod_dav_svn/repos.c Mon Jan 14 19:09:45 2002
>
> [...]
>
> + /* take the current data and shove it into the filter */
> + bb = apr_brigade_create(dc->pool);
> + bkt = apr_bucket_transient_create(buffer, *len);
> + APR_BRIGADE_INSERT_TAIL(bb, bkt);
> + if ((status = ap_pass_brigade(dc->output, bb)) != APR_SUCCESS) {
> + return svn_error_create(status, 0, NULL, dc->pool,
> + "Could not write data to filter.");
Is it important whether or not we use the APR_STATUS_IS_SUCCESS()
macro? If it is, I guess it should be used here. If it's not, I've
wasted a lot of keystrokes so far. :-)
> + /* done with the file. write an EOS bucket now. */
> + bb = apr_brigade_create(dc->pool);
> + bkt = apr_bucket_eos_create();
> + APR_BRIGADE_INSERT_TAIL(bb, bkt);
> + if ((status = ap_pass_brigade(dc->output, bb)) != APR_SUCCESS) {
Same.
> --- OLD/trunk/subversion/libsvn_ra_dav/fetch.c
> +++ NEW/trunk/subversion/libsvn_ra_dav/fetch.c
>
> [...]
>
> +#ifndef NEON_CTYPE_BUG
> + if (ne_version_minimum(0, 19)) /* if (api < 0.19) */
> + {
> + /* 0.18.x has a bug in the Content-Type parsing. work around it.
> + (subtype points to an empty string; one past is what we want
> + to look for) */
> +#if 0
> + printf("subtype=%s\n", cgc->ctype.subtype+1);
> +#endif
> + if (!strcmp(cgc->ctype.subtype+1, "nd.svn-svndiff"))
> + {
> + /* we are receiving an svndiff. set things up. */
> + frc->stream = svn_txdelta_parse_svndiff(frc->handler,
> + frc->handler_baton,
> + TRUE,
> + frc->pool);
> + }
> + }
> + else
> + /* ### need to patch this code when Neon is fixed */
> + abort();
> +#endif
Have you considered comments after #endifs indicating what they match,
e.g., "#endif /* NEON_CTYPE_BUG */"? Especially in nested cases like
the above, it makes it a lot easier to read.
> +
> cgc->checked_type = 1;
> }
>
> - data.data = (char *)buf;
> - data.len = len;
> - data.blocksize = len;
> - data.pool = frc->pool;
> -
> - op.action_code = svn_txdelta_new;
> - op.offset = 0;
> - op.length = len;
> -
> - window.tview_len = len; /* result will be this long */
> - window.num_ops = 1;
> - window.ops_size = 1; /* ### why is this here? */
> - window.ops = &op;
> - window.new_data = &data;
> - window.pool = frc->pool;
> -
> - /* We can't really do anything useful if we get an error here. Pass
> - it off to someone who can. */
> - cgc->err = (*frc->handler)(&window, frc->handler_baton);
> + if (frc->stream == NULL)
> + {
> + /* receiving plain text. construct a window for it. */
> +
> + svn_txdelta_window_t window = { 0 };
> + svn_txdelta_op_t op;
> + svn_stringbuf_t data;
> +
> + data.data = (char *)buf;
> + data.len = len;
> + data.blocksize = len;
> + data.pool = frc->pool;
> +
> + op.action_code = svn_txdelta_new;
> + op.offset = 0;
> + op.length = len;
> +
> + window.tview_len = len; /* result will be this long */
> + window.num_ops = 1;
> + window.ops_size = 1; /* ### why is this here? */
> + window.ops = &op;
> + window.new_data = &data;
> + window.pool = frc->pool;
> +
> + /* We can't really do anything useful if we get an error here. Pass
> + it off to someone who can. */
> + cgc->err = (*frc->handler)(&window, frc->handler_baton);
> + }
> + else
> + {
> + /* receiving svndiff. feed it to the svndiff parser. */
> +
> + apr_size_t written = len;
> +
> + cgc->err = svn_stream_write(frc->stream, buf, &written);
> +
> + /* ### the svndiff stream parser does not obey svn_stream semantics
> + ### in its write handler. it does not output the number of bytes
> + ### consumed by the handler. specifically, it may decrement the
> + ### number by 4 for the header, then never touch it again. that
> + ### makes it appear like an incomplete write.
> + ### disable this check for now. the svndiff parser actually does
> + ### consume all bytes, all the time.
> + */
Zounds. Glad you noticed that; I think Greg Hudson could fix it
fastest.
> +#if 0
> + if (written != len && cgc->err == NULL)
> + cgc->err = svn_error_createf(SVN_ERR_INCOMPLETE_DATA, 0, NULL,
> + frc->pool,
> + "unable to completely write the svndiff "
> + "data to the parser stream (wrote %lu "
> + "of %lu bytes)",
> + (unsigned long)written,
> + (unsigned long)len);
> +#endif
Same thing about the #endif.
Nice checkin! I can't wait to upgrade that server... No, make that "I
can wait a really long time to upgrade the server, until we're done
thoroughly testing the eol and keyword stuff." :-)
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:56 2006