Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] add xslt_stylesheet_param directive

Maxim Dounin
February 09, 2012 06:14PM
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
Subject Author Views Posted

[PATCH] add xslt_stylesheet_param directive Attachments

SamB 1656 January 24, 2012 05:04PM

Re: [PATCH] add xslt_stylesheet_param directive

Maxim Dounin 638 January 30, 2012 08:54PM

Re: [PATCH] add xslt_stylesheet_param directive Attachments

SamB 520 February 04, 2012 11:50AM

Re: [PATCH] add xslt_stylesheet_param directive

Laurence Rowe 578 February 07, 2012 08:04PM

Re: [PATCH] add xslt_stylesheet_param directive Attachments

SamB 513 February 08, 2012 07:58AM

Re: [PATCH] add xslt_stylesheet_param directive

Maxim Dounin 542 February 09, 2012 06:14PM

Re: [PATCH] add xslt_stylesheet_param directive

SamB 715 February 10, 2012 12:50PM

Re: [PATCH] add xslt_stylesheet_param directive

Maxim Dounin 642 February 09, 2012 06:14PM

Re: [PATCH] add xslt_stylesheet_param directive

SamB 675 February 10, 2012 12:02PM

Re: [PATCH] add xslt_stylesheet_param directive Attachments

SamB 607 February 26, 2012 10:18AM

Re: [PATCH] add xslt_stylesheet_param directive

Maxim Dounin 428 March 04, 2012 08:00AM

Re: [PATCH] add xslt_stylesheet_param directive

SamB 587 March 05, 2012 05:34PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 82
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready