[libxslt] Make xsl:sort thread-safe



commit e7374c3b9f1b1652862af1d37755611b00e4079e
Author: Nick Wellnhofer <wellnhofer aevum de>
Date:   Tue Oct 18 01:11:19 2022 +0200

    Make xsl:sort thread-safe
    
    xsl:sort with attribute value templates in data-type, order or lang
    attributes was never thread-safe since it temporarily modified the
    shared xsltStylePreComp structure.
    
    Fixes #78.

 examples/xsltICUSort.c |  74 +++++++++++----------------
 libxslt/xsltutils.c    | 136 +++++++++++++++++++++++--------------------------
 2 files changed, 93 insertions(+), 117 deletions(-)
---
diff --git a/examples/xsltICUSort.c b/examples/xsltICUSort.c
index 6f761214..0053122a 100644
--- a/examples/xsltICUSort.c
+++ b/examples/xsltICUSort.c
@@ -42,15 +42,14 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
     xmlXPathObjectPtr *resultsTab[XSLT_MAX_SORT];
     xmlXPathObjectPtr *results = NULL, *res;
     xmlNodeSetPtr list = NULL;
-    int descending, number, desc, numb;
     int len = 0;
     int i, j, incr;
     int tst;
     int depth;
     xmlNodePtr node;
     xmlXPathObjectPtr tmp;
-    xsltStylePreCompPtr comp;
-    int tempstype[XSLT_MAX_SORT], temporder[XSLT_MAX_SORT];
+    const xsltStylePreComp *comp;
+    int number[XSLT_MAX_SORT], desc[XSLT_MAX_SORT];
 
     /* Start ICU change */
     UCollator *coll = 0;
@@ -75,45 +74,46 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
 
     for (j = 0; j < nbsorts; j++) {
        comp = sorts[j]->_private;
-       tempstype[j] = 0;
        if ((comp->stype == NULL) && (comp->has_stype != 0)) {
-           comp->stype =
+           xmlChar *stype =
                xsltEvalAttrValueTemplate(ctxt, sorts[j],
                                          (const xmlChar *) "data-type",
                                          XSLT_NAMESPACE);
-           if (comp->stype != NULL) {
-               tempstype[j] = 1;
-               if (xmlStrEqual(comp->stype, (const xmlChar *) "text"))
-                   comp->number = 0;
-               else if (xmlStrEqual(comp->stype, (const xmlChar *) "number"))
-                   comp->number = 1;
+           number[j] = 0;
+           if (stype != NULL) {
+               if (xmlStrEqual(stype, (const xmlChar *) "text"))
+                   ;
+               else if (xmlStrEqual(stype, (const xmlChar *) "number"))
+                   number[j] = 1;
                else {
                    xsltTransformError(ctxt, NULL, sorts[j],
                          "xsltDoSortFunction: no support for data-type = %s\n",
-                                    comp->stype);
-                   comp->number = 0; /* use default */
+                         stype);
                }
+                xmlFree(stype);
            }
+        } else {
+            number[j] = comp->number;
        }
-       temporder[j] = 0;
        if ((comp->order == NULL) && (comp->has_order != 0)) {
-           comp->order = xsltEvalAttrValueTemplate(ctxt, sorts[j],
-                                                   (const xmlChar *) "order",
-                                                   XSLT_NAMESPACE);
-           if (comp->order != NULL) {
-               temporder[j] = 1;
-               if (xmlStrEqual(comp->order, (const xmlChar *) "ascending"))
-                   comp->descending = 0;
-               else if (xmlStrEqual(comp->order,
-                                    (const xmlChar *) "descending"))
-                   comp->descending = 1;
+           xmlChar *order = xsltEvalAttrValueTemplate(ctxt, sorts[j],
+                                                      BAD_CAST "order",
+                                                      XSLT_NAMESPACE);
+           desc[j] = 0;
+           if (order != NULL) {
+               if (xmlStrEqual(order, (const xmlChar *) "ascending"))
+                   ;
+               else if (xmlStrEqual(order, (const xmlChar *) "descending"))
+                   desc[j] = 1;
                else {
                    xsltTransformError(ctxt, NULL, sorts[j],
                             "xsltDoSortFunction: invalid value %s for order\n",
-                                    comp->order);
-                   comp->descending = 0; /* use default */
+                            order);
                }
+                xmlFree(order);
            }
+        } else {
+            desc[j] = comp->descending;
        }
     }
 
@@ -126,8 +126,6 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
     results = resultsTab[0];
 
     comp = sorts[0]->_private;
-    descending = comp->descending;
-    number = comp->number;
     if (results == NULL)
        return;
 
@@ -166,7 +164,7 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                if (results[j] == NULL)
                    tst = 1;
                else {
-                   if (number) {
+                   if (number[0]) {
                        if (results[j]->floatval == results[j + incr]->floatval)
                            tst = 0;
                        else if (results[j]->floatval >
@@ -186,7 +184,7 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                        tst = ucol_strcoll(coll, target, u_strlen(target), target2, u_strlen(target2));
                        /* End ICU change */
                    }
-                   if (descending)
+                   if (desc[0])
                        tst = -tst;
                }
                if (tst == 0) {
@@ -200,8 +198,6 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                        comp = sorts[depth]->_private;
                        if (comp == NULL)
                            break;
-                       desc = comp->descending;
-                       numb = comp->number;
 
                        /*
                         * Compute the result of the next level for the
@@ -216,7 +212,7 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                        if (res[j] == NULL)
                            tst = 1;
                        else {
-                           if (numb) {
+                           if (number[depth]) {
                                if (res[j]->floatval == res[j + incr]->floatval)
                                    tst = 0;
                                else if (res[j]->floatval >
@@ -236,7 +232,7 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                                tst = ucol_strcoll(coll, target, u_strlen(target), target2, 
u_strlen(target2));
                                /* End ICU change */
                            }
-                           if (desc)
+                           if (desc[depth])
                              tst = -tst;
                        }
                        /*
@@ -284,16 +280,6 @@ xsltICUSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
 
     for (j = 0; j < nbsorts; j++) {
        comp = sorts[j]->_private;
-       if (tempstype[j] == 1) {
-           /* The data-type needs to be recomputed each time */
-           xmlFree(comp->stype);
-           comp->stype = NULL;
-       }
-       if (temporder[j] == 1) {
-           /* The order needs to be recomputed each time */
-           xmlFree(comp->order);
-           comp->order = NULL;
-       }
        if (resultsTab[j] != NULL) {
            for (i = 0;i < len;i++)
                xmlXPathFreeObject(resultsTab[j][i]);
diff --git a/libxslt/xsltutils.c b/libxslt/xsltutils.c
index 9f0feb53..fc9bec60 100644
--- a/libxslt/xsltutils.c
+++ b/libxslt/xsltutils.c
@@ -947,10 +947,11 @@ xsltDocumentSortFunction(xmlNodeSetPtr list) {
 }
 
 /**
- * xsltComputeSortResultiInternal:
+ * xsltComputeSortResultInternal:
  * @ctxt:  a XSLT process context
- * @sort:  node list
- * @xfrm:  Transform strings according to locale
+ * @sort:  xsl:sort node
+ * @number:  data-type is number
+ * @locale:  transform strings according to locale
  *
  * reorder the current node list accordingly to the set of sorting
  * requirement provided by the array of nodes.
@@ -959,11 +960,11 @@ xsltDocumentSortFunction(xmlNodeSetPtr list) {
  */
 static xmlXPathObjectPtr *
 xsltComputeSortResultInternal(xsltTransformContextPtr ctxt, xmlNodePtr sort,
-                              int xfrm) {
+                              int number, xsltLocale locale) {
 #ifdef XSLT_REFACTORED
     xsltStyleItemSortPtr comp;
 #else
-    xsltStylePreCompPtr comp;
+    const xsltStylePreComp *comp;
 #endif
     xmlXPathObjectPtr *results = NULL;
     xmlNodeSetPtr list = NULL;
@@ -1031,10 +1032,10 @@ xsltComputeSortResultInternal(xsltTransformContextPtr ctxt, xmlNodePtr sort,
        if (res != NULL) {
            if (res->type != XPATH_STRING)
                res = xmlXPathConvertString(res);
-           if (comp->number)
+           if (number)
                res = xmlXPathConvertNumber(res);
            res->index = i;     /* Save original pos for dupl resolv */
-           if (comp->number) {
+           if (number) {
                if (res->type == XPATH_NUMBER) {
                    results[i] = res;
                } else {
@@ -1046,9 +1047,9 @@ xsltComputeSortResultInternal(xsltTransformContextPtr ctxt, xmlNodePtr sort,
                }
            } else {
                if (res->type == XPATH_STRING) {
-                   if ((xfrm) && (comp->locale != (xsltLocale)0)) {
+                   if (locale != (xsltLocale)0) {
                        xmlChar *str = res->stringval;
-                       res->stringval = (xmlChar *) xsltStrxfrm(comp->locale, str);
+                       res->stringval = (xmlChar *) xsltStrxfrm(locale, str);
                        xmlFree(str);
                    }
 
@@ -1088,7 +1089,8 @@ xsltComputeSortResultInternal(xsltTransformContextPtr ctxt, xmlNodePtr sort,
  */
 xmlXPathObjectPtr *
 xsltComputeSortResult(xsltTransformContextPtr ctxt, xmlNodePtr sort) {
-    return xsltComputeSortResultInternal(ctxt, sort, /* xfrm */ 0);
+    return xsltComputeSortResultInternal(ctxt, sort, /* number */ 0,
+                                         /* locale */ 0);
 }
 
 /**
@@ -1106,20 +1108,19 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
 #ifdef XSLT_REFACTORED
     xsltStyleItemSortPtr comp;
 #else
-    xsltStylePreCompPtr comp;
+    const xsltStylePreComp *comp;
 #endif
     xmlXPathObjectPtr *resultsTab[XSLT_MAX_SORT];
     xmlXPathObjectPtr *results = NULL, *res;
     xmlNodeSetPtr list = NULL;
-    int descending, number, desc, numb;
     int len = 0;
     int i, j, incr;
     int tst;
     int depth;
     xmlNodePtr node;
     xmlXPathObjectPtr tmp;
-    int tempstype[XSLT_MAX_SORT], temporder[XSLT_MAX_SORT],
-        templang[XSLT_MAX_SORT];
+    int number[XSLT_MAX_SORT], desc[XSLT_MAX_SORT];
+    xsltLocale locale[XSLT_MAX_SORT];
 
     if ((ctxt == NULL) || (sorts == NULL) || (nbsorts <= 0) ||
        (nbsorts >= XSLT_MAX_SORT))
@@ -1136,71 +1137,70 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
 
     for (j = 0; j < nbsorts; j++) {
        comp = sorts[j]->psvi;
-       tempstype[j] = 0;
        if ((comp->stype == NULL) && (comp->has_stype != 0)) {
-           comp->stype =
+           xmlChar *stype =
                xsltEvalAttrValueTemplate(ctxt, sorts[j],
-                                         (const xmlChar *) "data-type",
-                                         NULL);
-           if (comp->stype != NULL) {
-               tempstype[j] = 1;
-               if (xmlStrEqual(comp->stype, (const xmlChar *) "text"))
-                   comp->number = 0;
-               else if (xmlStrEqual(comp->stype, (const xmlChar *) "number"))
-                   comp->number = 1;
+                                         BAD_CAST "data-type", NULL);
+           number[j] = 0;
+           if (stype != NULL) {
+               if (xmlStrEqual(stype, (const xmlChar *) "text"))
+                   ;
+               else if (xmlStrEqual(stype, (const xmlChar *) "number"))
+                   number[j] = 1;
                else {
                    xsltTransformError(ctxt, NULL, sorts[j],
                          "xsltDoSortFunction: no support for data-type = %s\n",
-                                    comp->stype);
-                   comp->number = 0; /* use default */
+                         stype);
                }
+                xmlFree(stype);
            }
-       }
-       temporder[j] = 0;
+       } else {
+           number[j] = comp->number;
+        }
        if ((comp->order == NULL) && (comp->has_order != 0)) {
-           comp->order = xsltEvalAttrValueTemplate(ctxt, sorts[j],
-                                                   (const xmlChar *) "order",
-                                                   NULL);
-           if (comp->order != NULL) {
-               temporder[j] = 1;
-               if (xmlStrEqual(comp->order, (const xmlChar *) "ascending"))
-                   comp->descending = 0;
-               else if (xmlStrEqual(comp->order,
-                                    (const xmlChar *) "descending"))
-                   comp->descending = 1;
+           xmlChar *order = xsltEvalAttrValueTemplate(ctxt, sorts[j],
+                                                       BAD_CAST "order", NULL);
+           desc[j] = 0;
+           if (order != NULL) {
+               if (xmlStrEqual(order, (const xmlChar *) "ascending"))
+                   ;
+               else if (xmlStrEqual(order, (const xmlChar *) "descending"))
+                   desc[j] = 1;
                else {
                    xsltTransformError(ctxt, NULL, sorts[j],
                             "xsltDoSortFunction: invalid value %s for order\n",
-                                    comp->order);
-                   comp->descending = 0; /* use default */
+                            order);
                }
+                xmlFree(order);
            }
+       } else {
+           desc[j] = comp->descending;
        }
-       templang[j] = 0;
        if ((comp->lang == NULL) && (comp->has_lang != 0)) {
             xmlChar *lang = xsltEvalAttrValueTemplate(ctxt, sorts[j],
                                                      (xmlChar *) "lang",
                                                      NULL);
            if (lang != NULL) {
-               templang[j] = 1;
-                comp->locale = xsltNewLocale(lang);
+                locale[j] = xsltNewLocale(lang);
                 xmlFree(lang);
+            } else {
+                locale[j] = 0;
             }
-       }
+       } else {
+            locale[j] = comp->locale;
+        }
     }
 
     len = list->nodeNr;
 
-    resultsTab[0] = xsltComputeSortResultInternal(ctxt, sorts[0],
-                                                  /* xfrm */ 1);
+    resultsTab[0] = xsltComputeSortResultInternal(ctxt, sorts[0], number[0],
+                                                  locale[0]);
     for (i = 1;i < XSLT_MAX_SORT;i++)
        resultsTab[i] = NULL;
 
     results = resultsTab[0];
 
     comp = sorts[0]->psvi;
-    descending = comp->descending;
-    number = comp->number;
     if (results == NULL)
        goto cleanup;
 
@@ -1215,7 +1215,7 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                if (results[j] == NULL)
                    tst = 1;
                else {
-                   if (number) {
+                   if (number[0]) {
                        /* We make NaN smaller than number in accordance
                           with XSLT spec */
                        if (xmlXPathIsNaN(results[j]->floatval)) {
@@ -1232,16 +1232,16 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                                results[j + incr]->floatval)
                            tst = 1;
                        else tst = -1;
-                   } else if(comp->locale != (xsltLocale)0) {
+                   } else if(locale[0] != (xsltLocale)0) {
                        tst = xsltLocaleStrcmp(
-                           comp->locale,
+                           locale[0],
                            (xsltLocaleChar *) results[j]->stringval,
                            (xsltLocaleChar *) results[j + incr]->stringval);
                    } else {
                        tst = xmlStrcmp(results[j]->stringval,
                                     results[j + incr]->stringval);
                    }
-                   if (descending)
+                   if (desc[0])
                        tst = -tst;
                }
                if (tst == 0) {
@@ -1255,8 +1255,6 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                        comp = sorts[depth]->psvi;
                        if (comp == NULL)
                            break;
-                       desc = comp->descending;
-                       numb = comp->number;
 
                        /*
                         * Compute the result of the next level for the
@@ -1266,7 +1264,8 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                            resultsTab[depth] =
                                 xsltComputeSortResultInternal(ctxt,
                                                               sorts[depth],
-                                                              /* xfrm */ 1);
+                                                              number[depth],
+                                                              locale[depth]);
                        res = resultsTab[depth];
                        if (res == NULL)
                            break;
@@ -1276,7 +1275,7 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                        } else if (res[j+incr] == NULL) {
                            tst = -1;
                        } else {
-                           if (numb) {
+                           if (number[depth]) {
                                /* We make NaN smaller than number in
                                   accordance with XSLT spec */
                                if (xmlXPathIsNaN(res[j]->floatval)) {
@@ -1295,16 +1294,16 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
                                        res[j + incr]->floatval)
                                    tst = 1;
                                else tst = -1;
-                           } else if(comp->locale != (xsltLocale)0) {
+                           } else if(locale[depth] != (xsltLocale)0) {
                                tst = xsltLocaleStrcmp(
-                                   comp->locale,
+                                   locale[depth],
                                    (xsltLocaleChar *) res[j]->stringval,
                                    (xsltLocaleChar *) res[j + incr]->stringval);
                            } else {
                                tst = xmlStrcmp(res[j]->stringval,
                                             res[j + incr]->stringval);
                            }
-                           if (desc)
+                           if (desc[depth])
                                tst = -tst;
                        }
 
@@ -1349,20 +1348,11 @@ xsltDefaultSortFunction(xsltTransformContextPtr ctxt, xmlNodePtr *sorts,
 cleanup:
     for (j = 0; j < nbsorts; j++) {
        comp = sorts[j]->psvi;
-       if (tempstype[j] == 1) {
-           /* The data-type needs to be recomputed each time */
-           xmlFree((void *)(comp->stype));
-           comp->stype = NULL;
-       }
-       if (temporder[j] == 1) {
-           /* The order needs to be recomputed each time */
-           xmlFree((void *)(comp->order));
-           comp->order = NULL;
-       }
-       if (templang[j] == 1) {
-           xsltFreeLocale(comp->locale);
-           comp->locale = (xsltLocale)0;
-       }
+       if ((comp->lang == NULL) && (comp->has_lang != 0)) {
+            if (locale[j] != (xsltLocale)0) {
+                xsltFreeLocale(locale[j]);
+            }
+        }
        if (resultsTab[j] != NULL) {
            for (i = 0;i < len;i++)
                xmlXPathFreeObject(resultsTab[j][i]);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]