re: dlls/msxml3: Using strlenW instead of SysStringLen to check node name length
Hi Eric, welcome to Wine! http://msdn.microsoft.com/en-us/library/ms221240.aspx says the only time those two functions disagree on valid bstr's is if there are embedded nul chars, in which case SysStringLength probably returns a larger value. Are you quite, quite sure that your bstrs are valid? (In general, the best way to answer questions like that is to add a test case to your patch, and make sure it passes on Windows as well as Wine before submitting to wine-patches.) - Dan
On 3/23/2010 16:21, Dan Kegel wrote:
Hi Eric, welcome to Wine! http://msdn.microsoft.com/en-us/library/ms221240.aspx says the only time those two functions disagree on valid bstr's is if there are embedded nul chars, in which case SysStringLength probably returns a larger value. Hi Dan.
It's a quite obvious thing. You can use BSTR in place of any WCHAR based parameter, keeping in mind that nul inside BSTR will be treated as end of string, and BSTR itself is always nul-terminated. In this case embedded nulls aren't allowed for node names I suppose, at least http://www.w3.org/TR/REC-xml/#NT-Name tells that it's not possible. The problem here is that native implementation most likely allows this parameter to be WCHAR string, and this is a case for ::loadXML() method (already fixed). It's not a big deal to fix it, with tests of course.
Are you quite, quite sure that your bstrs are valid?
Most likely they are not, which indicates an application bug or our BSTR management deficiency, I believe there was a discussion in bugzilla about some caching functionality for example we don't provide, not sure it could be related, just as an example. Eric, please show us a code you're using to create BSTR before passing it to msxml.
(In general, the best way to answer questions like that is to add a test case to your patch, and make sure it passes on Windows as well as Wine before submitting to wine-patches.)
I already asked for that.
- Dan
Sure thing, The line we call is: m_pMD5XMLDoc->createNode(varType, L"md5report", L"", &pMD5RootNode); So I think we are sending the createNode function WCHAR strings like Nikolay is saying. For that call the SysStringLen function is giving a length of 0 for L"md5report". I think that Nikolay's code in LoadXML() handles our use-case properly, my only concern is that the code that is giving me a problem is meant to be an early-exit and I'm thinking that if we add so much checking code to it then it defeats the purpose of the early exit. Eric On Mar 23, 2010, at 9:40 AM, Nikolay Sivov wrote:
On 3/23/2010 16:21, Dan Kegel wrote:
Hi Eric, welcome to Wine! http://msdn.microsoft.com/en-us/library/ms221240.aspx says the only time those two functions disagree on valid bstr's is if there are embedded nul chars, in which case SysStringLength probably returns a larger value. Hi Dan.
It's a quite obvious thing. You can use BSTR in place of any WCHAR based parameter, keeping in mind that nul inside BSTR will be treated as end of string, and BSTR itself is always nul-terminated. In this case embedded nulls aren't allowed for node names I suppose, at least http://www.w3.org/TR/REC-xml/#NT-Name tells that it's not possible.
The problem here is that native implementation most likely allows this parameter to be WCHAR string, and this is a case for ::loadXML() method (already fixed). It's not a big deal to fix it, with tests of course.
Are you quite, quite sure that your bstrs are valid?
Most likely they are not, which indicates an application bug or our BSTR management deficiency, I believe there was a discussion in bugzilla about some caching functionality for example we don't provide, not sure it could be related, just as an example.
Eric, please show us a code you're using to create BSTR before passing it to msxml.
(In general, the best way to answer questions like that is to add a test case to your patch, and make sure it passes on Windows as well as Wine before submitting to wine-patches.)
I already asked for that.
- Dan
On 3/23/2010 16:48, Eric Lanz wrote:
Sure thing,
The line we call is:
m_pMD5XMLDoc->createNode(varType, L"md5report", L"",&pMD5RootNode);
So it's not a BSTR.
So I think we are sending the createNode function WCHAR strings like Nikolay is saying.
For that call the SysStringLen function is giving a length of 0 for L"md5report".
You are lucky that it doesn't crash.
I think that Nikolay's code in LoadXML() handles our use-case properly, my only concern is that the code that is giving me a problem is meant to be an early-exit and I'm thinking that if we add so much checking code to it then it defeats the purpose of the early exit.
No sure what you mean here. This one is more complex:
if (!name || SysStringLen(name) == 0) return E_FAIL;
comparing with what you need:
if (!name || !*name) return E_FAIL;
or simplifying:
if (!(name && *name)) return E_FAIL; Eric
participants (3)
-
Dan Kegel -
Eric Lanz -
Nikolay Sivov