Hello!
On Wed, Feb 08, 2012 at 01:55:47PM +0100, SamB wrote:
> Hi,
>
> I've added xslt_string_param and slightly changed parameter passing and
> transformation
> to make it work as expected.
> As a side effect of this change, might be usage of
> xslt_param/xslt_string_param faster,
> because params are directly inserted to transformation context and do
> not need to be
> copied to name/value parameter array that is passed to
> xsltApplyStylesheet() as it's now
> by parameters passed via xslt_stylesheet directive.
>
> Old parameter passing using xslt_stylesheet has not been changed to
> prevent unexpected behavior,
> maybe this can be modified later if this patch will prove to be stable...
This looks intresting, though may be it would be better to finish
and commit the xslt_param patch first (see another message for a
review).
Below is review of the xslt_param + xslt_string_param patch
specific bits.
[...]
> diff -ur nginx-1.1.13/src/http/modules/ngx_http_xslt_filter_module.c nginx-1.1.13-new/src/http/modules/ngx_http_xslt_filter_module.c
> --- nginx-1.1.13/src/http/modules/ngx_http_xslt_filter_module.c 2010-07-12 14:52:01.000000000 +0200
> +++ nginx-1.1.13-new/src/http/modules/ngx_http_xslt_filter_module.c 2012-02-08 13:34:43.000000000 +0100
> @@ -10,6 +10,7 @@
>
> #include <libxml/parser.h>
> #include <libxml/tree.h>
> +#include <libxslt/variables.h>
> #include <libxslt/xslt.h>
> #include <libxslt/xsltInternals.h>
> #include <libxslt/transform.h>
> @@ -48,6 +49,8 @@
> ngx_array_t sheets; /* ngx_http_xslt_sheet_t */
> ngx_hash_t types;
> ngx_array_t *types_keys;
> + ngx_array_t string_params; /* ngx_http_complex_value_t */
> + ngx_array_t params; /* ngx_http_complex_value_t */
I would like to see single "params" array instead, with both
string params and xpath ones.
[...]
> @@ -84,6 +90,13 @@
> void *conf);
> static char *ngx_http_xslt_stylesheet(ngx_conf_t *cf, ngx_command_t *cmd,
> void *conf);
> +static char *ngx_http_xslt_add_param(ngx_conf_t *cf, ngx_command_t *cmd,
> + ngx_array_t *params);
> +static char *ngx_http_xslt_stylesheet_param(ngx_conf_t *cf, ngx_command_t *cmd,
> + void *conf);
> +static char *ngx_http_xslt_stylesheet_string_param(ngx_conf_t *cf,
> + ngx_command_t *cmd,
> + void *conf);
There is no need for 3 functions here. Just one is enough.
In your current code, you may (and probably have to) use "offset"
field of the command structure to pass offset of array you want to
work with. With single params array as suggested earlier - you
may use "post" field to distinguish string and non-string
versions.
> static void ngx_http_xslt_cleanup_dtd(void *data);
> static void ngx_http_xslt_cleanup_stylesheet(void *data);
> static void *ngx_http_xslt_filter_create_main_conf(ngx_conf_t *cf);
> @@ -116,6 +129,20 @@
> 0,
> NULL },
>
> + { ngx_string("xslt_param"),
> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
> + ngx_http_xslt_stylesheet_param,
> + NGX_HTTP_LOC_CONF_OFFSET,
> + 0,
> + NULL },
> +
> + { ngx_string("xslt_string_param"),
> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2,
> + ngx_http_xslt_stylesheet_string_param,
> + NGX_HTTP_LOC_CONF_OFFSET,
> + 0,
> + NULL },
> +
> { ngx_string("xslt_types"),
> NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_1MORE,
> ngx_http_types_slot,
> @@ -450,6 +477,7 @@
> ngx_uint_t i;
> xmlChar *buf;
> xmlDocPtr doc, res;
> + xsltTransformContextPtr xctxt;
In libxslt this is usually called "ctxt", I think we should
follow.
> ngx_http_xslt_sheet_t *sheet;
> ngx_http_xslt_filter_loc_conf_t *conf;
>
> @@ -468,14 +496,28 @@
>
> for (i = 0; i < conf->sheets.nelts; i++) {
>
> - if (ngx_http_xslt_params(r, ctx, &sheet[i].params) != NGX_OK) {
> + xctxt = xsltNewTransformContext(sheet[i].stylesheet, doc);
> + if (xctxt == NULL) {
> xmlFreeDoc(doc);
> return NULL;
> }
>
> - res = xsltApplyStylesheet(sheet[i].stylesheet, doc, ctx->params.elts);
> + xctxt->initialContextDoc = doc;
> + xctxt->initialContextNode = (xmlNodePtr) doc;
According to
http://xmlsoft.org/XSLT/html/libxslt-xsltInternals.html
this is internal to libxslt, and using it may not be a good idea.
Are you sure it's needed?
As far as I can see in sources, it's ceratainly *not* needed for
xsltQuoteOneUserParam(). Not sure about xsltEvalOneUserParam().
> +
> + if (ngx_http_xslt_params(r, ctx, conf, xctxt,
> + &sheet[i].params) != NGX_OK) {
> + xmlFreeDoc(doc);
> + xsltFreeTransformContext(xctxt);
It may be a good idea to free this in reverse order, i.e.
transformation context first.
> + return NULL;
> + }
> +
> + res = xsltApplyStylesheetUser(sheet[i].stylesheet, doc,
> + ctx->params.elts,
> + NULL, NULL, xctxt);
>
> xmlFreeDoc(doc);
> + xsltFreeTransformContext(xctxt);
Same here.
>
> if (res == NULL) {
> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> @@ -564,14 +606,92 @@
[...]
Maxim Dounin
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
