Re: [xslt] Patch for xsl:sort lang support

[Nick Wellnhofer <wellnhofer aevum de>]

Roumen Petrov wrote:
> Roumen Petrov wrote:
> > Daniel Veillard wrote:
> > > On Tue, Jun 10, 2008 at 02:05:12AM +0200, Nick Wellnhofer wrote:
> > > > Roumen Petrov wrote:
> > > > > Nick Wellnhofer wrote:
> > > > > > Roumen Petrov wrote:
> > > > > > > Please find patch that is first attempt to avoid build problem 
> > > > > > > after recent commits related to xsltlocale.
> > > > > > >
> > > > > > > I'm not convinced that xsltlocale.c is correct for msvcrt:
> > > > > > > The manuals show that rfc3066 language tags are supported but the 
> > > > > > > current code ignore all of them.
> > > > > > I took the language codes from here:
> > > > > > http://msdn.microsoft.com/en-us/library/39cwe7zf(VS.80).aspx
> > > > > 10x. I see that the _create_locale is related to setlocale.
> > > > Yes, _create_locale accepts the same parameters as setocale.
> > > >
> > > > > On this page 
> > > > > http://msdn.microsoft.com/en-us/library/ms776260(VS.85).aspx
> > > > > is listed another information but for other functions.
> > > > > May be win32 locale has to be based on this.
> > > > No, the CRT functions don't seem to accept those language codes.
> > > >
> > > > > > > Also I think that _create_locale is missing before msvcrt80. The 
> > > > > > > proposed patch don't resolve this in makefile based win32 build.
> > > > > > You're right, it seems it was introduced with VC8. Maybe it would 
> > > > > > be best to have a configure switch to disable locale support in 
> > > > > > case you want a build that runs without msvcr80.dll?
> > > > > In the proposed patch configure will try detect presence of 
> > > > > _create_locale if don't exist header xlocale.h , but not in 
> > > > > makefile based build ( win32/Makefile.msvc ). May a new makefile 
> > > > > win32/Makefile.msvcXX can ersolve problem.
> > > > Your patch looks good. See the attached version (also against 
> > > > current trunk) that additionally moves the defines to 
> > > > libxslt/xsltconfig.h and adds a switch to win32/configure.js.
> > > >
> > > > I only wonder which setup uses GNU make and MSVCRT.
> > > >
> > > > > > > I wonder why the implementation assume that if exist xlocale.h 
> > > > > > > the charmap is ascii.
> > > > > > Which charmap do you mean?
> > > > > EBCDIC
> > > > I didn't think about that. Maybe one should add an appropriate 
> > > > #ifdef somewhere...
> > >
> > >   Okay I applied and commited this patch which seems to integrate ideas
> > > from both of you (i.e. i assume it superseeds Roumen's own 
> > > libxslt-trunk-20080608.patch , right ?)
> > yes, it is fine with me (only code review).
> > (sorry for late reply but I'm too busy with other task and i don't 
> > have time to test).
> >
> > >   Is there anything still pending on this ?
> >
> > I'm thinking about more backward compatible w32 implementation since 
> > the current require msvcrt80.
> > The xlocale based sort works fine.
> > But the related bug can be closed.
> >
> > > Daniel
> >
> > Roumen
>
> Please find attached file "libxslt-trunk-20080625.diff.gz" with w32 
> implementation based on winapi functions. It has to work on all 
> NTs(nt4/w2k/xp/..). With some link quirks(not in the patch) it has to 
> work on 9x(95/98/me).
>
> Roumen

Looks very nice. I needed one little fix to get it to compile with the 
Microsoft C compiler and nmake. See the attached patch (on top of your 
patch) that also fixes two compiler warnings.

We might also want to keep the following checks in xsltLocaleStrCmp

if (str1 == str2) return(0);
if (str1 == NULL) return(-1);
if (str2 == NULL) return(1);

so that it behaves like xmlStrcmp with regard to NULL pointers.

Nick

--- libxslt/libxslt/win32config.h	2008-06-28 22:50:53.726600000 +0200
+++ libxslt-nik/libxslt/win32config.h	2008-06-28 23:00:51.518600000 +0200
@@ -95,5 +95,7 @@ static int isnan (double d) {
 #define ATTRIBUTE_UNUSED
 #endif
 
+#define _WINSOCKAPI_
+
 #endif /* __LIBXSLT_WIN32_CONFIG__ */
 
--- libxslt/libxslt/preproc.c	2008-06-28 22:50:53.352200000 +0200
+++ libxslt-nik/libxslt/preproc.c	2008-06-28 22:17:39.469400000 +0200
@@ -489,7 +489,7 @@ xsltFreeStylePreComp(xsltStylePreCompPtr
 	    break;
     }
 #else    
-    if (comp->locale != NULL)
+    if (comp->locale != (xsltLocale)0)
 	xsltFreeLocale(comp->locale);
     if (comp->comp != NULL)
 	xmlXPathFreeCompExpr(comp->comp);
@@ -736,7 +736,7 @@ xsltSortComp(xsltStylesheetPtr style, xm
 	comp->locale = xsltNewLocale(comp->lang);
     }
     else {
-        comp->locale = NULL;
+        comp->locale = (xsltLocale)0;
     }
 
     comp->select = xsltGetCNsProp(style, inst,(const xmlChar *)"select", XSLT_NAMESPACE);