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@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