[3/3] msxml3/domdoc: Add support for VT_ARRAY|VT_UI1 in domdoc_load(). (try 2)

Nikolay Sivov bunglehead at gmail.com
Thu May 5 09:37:14 CDT 2011


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.




More information about the wine-devel mailing list