dberlin@tigris.org writes:
> Author: dberlin
> Date: Wed Nov 9 11:51:14 2005
> New Revision: 17273
> +static int sort_encoding_pref(const void *accept_rec1, const void *accept_rec2)
> +{
> + float diff = ((accept_rec *) accept_rec1)->quality -
> + ((accept_rec *) accept_rec2)->quality;
I cast to (const accept_rec *) to avoid dropping const.
> + return (diff == 0 ? 0 : (diff > 0 ? -1 : 1));
> +}
> +
> +/* It would be nice if this function could be unit-tested. Paul
> + Querna suggested
> + http://svn.apache.org/repos/asf/httpd/httpd/trunk/server/request.c's
> + make_sub_request(), and noted that httpd manually constructs
> + request_rec's in a few spots. */
> +
> +static svn_error_t *
> +svn_dav__negotiate_encoding_prefs(request_rec *r,
> + int *svndiff_version)
> +{
> + /* It would be nice if mod_negotiation
> + <http://httpd.apache.org/docs-2.1/mod/mod_negotiation.html> could
> + handle the Accept-Encoding header parsing for us. Sadly, it
> + looks like its data structures and routines are private (see
> + httpd/modules/mappers/mod_negotiation.c). Thus, we duplicate the
> + necessary ones in this file. */
> + size_t i;
> + const apr_array_header_t *encoding_prefs;
> + encoding_prefs = do_header_line(r->pool,
> + apr_table_get(r->headers_in,
> + "Accept-Encoding"));
> +
> + if (apr_is_empty_array(encoding_prefs))
do_header_line can return NULL; now it appears that apr_is_empty_array
handle will handle NULL but I had to look at the implementation rather
than the documentation to determine that. I don't know if that's an
APR bug or a Subversion bug.
> + {
> + *svndiff_version = 0;
> + return SVN_NO_ERROR;
> + }
> +
> + qsort(encoding_prefs->elts, (size_t) encoding_prefs->nelts,
> + sizeof(accept_rec), sort_encoding_pref);
> + for (i = 0; i < encoding_prefs->nelts; i++)
> + {
> + struct accept_rec rec = APR_ARRAY_IDX (encoding_prefs, i,
> + struct accept_rec);
> + if (strcmp (rec.name, "svndiff1") == 0)
> + {
> + *svndiff_version = 1;
> + break;
> + }
> + else if (strcmp (rec.name, "svndiff") == 0)
> + {
> + *svndiff_version = 0;
> + break;
> + }
> + }
> + return SVN_NO_ERROR;
If neither strcmp matches *svndiff_version does not get set, is that
intended? If so it should be documented.
> +}
>
>
> static dav_error * dav_svn_get_resource(request_rec *r,
> @@ -1334,6 +1512,7 @@
> && strcmp(ct, SVN_SVNDIFF_MIME_TYPE) == 0;
> }
>
> + svn_dav__negotiate_encoding_prefs (r, &comb->priv.svndiff_version);
That would leak an error if svn_dav__negotiate_encoding_prefs ever
returned one, since it never does perhaps it should be void?
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 10 01:31:41 2005