mshtml: Added IHTMLTable::width property implementation. (try 4)

[Jacek Caban]

On 04/26/14 15:20, Zhenbo Li wrote:
> Thank you.
> I'm sorry for the mistakes I've made.
>
> As this patch still needs improving, maybe mark 104193, 104110, 103576
> as rejected?

Don't worry too much about them being marked one way or another.

> 2014-04-26 20:44 GMT+08:00 Jacek Caban <jacek at codeweavers.com>:
>> Hi Zhenbo,
>>
>> This is better, but:
>>
>> On 04/26/14 05:12, Zhenbo Li wrote:
>>
>> +    hres = var2str(&v, &val);
>> +
>> +    if (hres != S_OK){
>> +        ERR("Set Width(%s) failed when initializing a nsAString!\n",
>> +            debugstr_variant(&v));
>> +        nsAString_Finish(&val);
>>
>>
>> Again, this is not initialized in the error case.
> As I've added
>> +    default:
>> +        nsAString_Init(nsstr, NULL);
>> +        FIXME("unsupported arg %s\n", debugstr_variant(p));
>> +        return E_NOTIMPL;
>> +    }
> Isn't it enough?

I missed that change, but I think it's wrong. There are other error
cases as well. nsAString_Init failure also leaves the string
uninitialized (and it's on my TODO list to make sure that we may make
nsAString_Init infailable, but that's a different story). It's also
generally sane to expect that if a function fails, it's output parameter
is not initialized. Please remove this new nsAString_Init call and
nsAString_Finish in error case.

> Or you mean here?
>> +    ret = nsAString_Init(nsstr, str);
>> +    SysFreeString(str);
>> +    return ret ? S_OK : E_OUTOFMEMORY;
> Should I free a string which failed when initializing due to out-of-memory?

Which one? The way it's done currently is right.

>> +
>> +    hres = nsstr_to_truncated_bstr(&val, &bstr);
>> +    if (FAILED(hres)) {
>> +        SysFreeString(bstr);
>>
>>
>> And bstr is not initialized in error case.
> The error case comes from
>> +    ret = end ? SysAllocStringLen(str, end-str) : SysAllocString(str);
>> +    if(!ret)
>> +        return E_OUTOFMEMORY;
> I have the same question.

Please read the code again. bstr is not initialized in this case, so you
can't free it.

Jacek