On 5/5/2011 18:14, Adam Martinson wrote:
On 05/04/2011 06:04 PM, Nikolay Sivov wrote:
On 5/5/2011 00:38, Adam Martinson wrote:
Fixes bugs 14864 + 16453. --- dlls/msxml3/domdoc.c | 27 +++++++++++++ dlls/msxml3/tests/domdoc.c | 94 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 1 deletions(-) + case VT_ARRAY|VT_UI1: + { + SAFEARRAY *psa = V_ARRAY(&source); + xmlChar *str; + LONG len; + UINT dim = SafeArrayGetDim(psa); + + switch (dim) + { + case 0: + ERR("SAFEARRAY == NULL\n"); + hr = E_INVALIDARG; + break; + case 1: + /* Only takes UTF8 strings. + * NOT NULL-terminated. */ + SafeArrayAccessData(psa, (void**)&str); + SafeArrayGetUBound(psa, 1,&len); + hr = load_utf8(This, str, ++len, isSuccessful); + SafeArrayUnaccessData(psa); + break; + default: + FIXME("unhandled SAFEARRAY dim: %d\n", dim); + hr = E_NOTIMPL; + } + }
This could be simplified. SafeArrayGetUBound will fail for case 0 for example, and unhandled dimension should be WARN, IMO. I think this is actually the most straightforward/readable way to handle it. If multi-dimensional SAFEARRAYs are actually supported, we will need separate cases for them. It's just not the way I thought about it, that's all. Ok, let it be like that, but could you remove ERR() from it? I feel like it will be useless line that never works, on the other hand we should add dumping of array dimension in debugstr_variant() so it will be instantly visible in trace. And IMHO if an app is trying to do something we don't handle, and don't know how it should be handled, it should be a FIXME. Well ok, I don't really expect it to come often.