Alexandre Goujon ale.goujon@gmail.com writes:
@@ -181,6 +181,30 @@ static inline BSTR bstr_from_xmlChar(const xmlChar *str) return ret; }
+static inline BSTR bstr_trim(const BSTR str) +{
- UINT i, j, len = SysStringLen(str);
- BSTR new_str = NULL;
- /* str empty or NULL */
- if(!len)
goto err;
- for(i=0; isspaceW(str[i]) && i<len; i++);
- for(j=len-1; isspaceW(str[j]) && j>=i; j--);
- new_str = SysAllocStringLen(str+i, j-i+1);
- if(!new_str)
goto err;
- SysFreeString(str);
- return new_str;
- err:
SysFreeString(str);
return SysAllocStringLen(NULL, 0);
+}
That's ugly and inefficient, especially since the common case is to not modify it.
On 09/02/2010 12:10 PM, Alexandre Julliard wrote:
That's ugly and inefficient, especially since the common case is to not modify it.
Well, in fact I don't modify the content. I just trim the output of bstr_from_xmlChar( xmlNodeGetContent( (xmlNodePtr)This->node ) ) And I made a separate function to not overload xml_get_text.
Anyway, I understand it's not very clear as the error message is currently "Test marked todo: incorrect get_text string". I'll send a patch to change and add "got %s", wine_dbgstr_w(str) to it. I think it'll be more .. obvious.
On Thu, Sep 2, 2010 at 1:52 PM, GOUJON Alexandre ale.goujon@gmail.comwrote:
On 09/02/2010 12:10 PM, Alexandre Julliard wrote:
That's ugly and inefficient, especially since the common case is to not modify it.
Well, in fact I don't modify the content. I just trim the output of bstr_from_xmlChar( xmlNodeGetContent( (xmlNodePtr)This->node ) ) And I made a separate function to not overload xml_get_text.
I think what Alexandre J means is that you are reallocating the buffer every time (even if there is no whitespace to trim), which is inefficient for the average case (in which there will be no whitespace to trim). You only have to reallocate the string if (j - i < len - 1).
You could also try this hack: you don't have to reallocate the string even if it has whitespace, use memmove to get rid of the leading whitespace, then just set the string's length to j - i + 1.
Not sure if it makes sense to reallocate the string if it's empty.
Octavian
On 9/2/2010 14:10, Alexandre Julliard wrote:
Alexandre Goujonale.goujon@gmail.com writes:
@@ -181,6 +181,30 @@ static inline BSTR bstr_from_xmlChar(const xmlChar *str) return ret; }
+static inline BSTR bstr_trim(const BSTR str) +{
- UINT i, j, len = SysStringLen(str);
- BSTR new_str = NULL;
- /* str empty or NULL */
- if(!len)
goto err;
- for(i=0; isspaceW(str[i])&& i<len; i++);
- for(j=len-1; isspaceW(str[j])&& j>=i; j--);
- new_str = SysAllocStringLen(str+i, j-i+1);
- if(!new_str)
goto err;
- SysFreeString(str);
- return new_str;
- err:
SysFreeString(str);
return SysAllocStringLen(NULL, 0);
+}
That's ugly and inefficient, especially since the common case is to not modify it.
Also this probably depends on ::preserveWhiteSpace() property.
On 09/03/2010 06:52 AM, Nikolay Sivov wrote:
Also this probably depends on ::preserveWhiteSpace() property.
You are talking about the 'preserving' property from the internal domdoc structure, aren't you ? I tried accessing it from the xmlnode with xmlnode_get_ownerDocument but then I can't do anything as the domdoc structure is defined in domdoc.c not in node.c.
Any hint ? Sorry if it's a silly question, I'm still learning.
On 9/4/2010 15:37, GOUJON Alexandre wrote:
On 09/03/2010 06:52 AM, Nikolay Sivov wrote:
Also this probably depends on ::preserveWhiteSpace() property.
You are talking about the 'preserving' property from the internal domdoc structure, aren't you ? I tried accessing it from the xmlnode with xmlnode_get_ownerDocument but then I can't do anything as the domdoc structure is defined in domdoc.c not in node.c.
Any hint ? Sorry if it's a silly question, I'm still learning.
It's not silly at all. Currently there's no way to do it, and I'm thinking about making it possible, it's needed for other global properties too.
On Thu, Sep 02, 2010 at 12:10:26PM +0200, Alexandre Julliard wrote:
Alexandre Goujon ale.goujon@gmail.com writes:
+static inline BSTR bstr_trim(const BSTR str) +{
...
+}
That's ugly and inefficient, especially since the common case is to not modify it.
Not to mention the bloatage of using 'inline' for something that size.
David
On 09/05/2010 11:52 AM, David Laight wrote:
Not to mention the bloatage of using 'inline' for something that size
I modified my patch accordingly but as I can't check the preserveWhiteSpace property, I have to wait.
Anyway, thanks for all your advices !