- str = SysAllocStringLen(NULL, 1023);
- newstr = SysAllocStringLen(NULL, 1023);
Where this length comes from?
- switch(V_VT(arg + 1)) {
- case VT_NULL:
return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE);- case VT_BSTR:
str = V_BSTR(arg + 1);break;- case VT_ARRAY|VT_BYREF|VT_VARIANT:
return DISP_E_TYPEMISMATCH;- default:
hres = to_short(arg + 1, &tmp);if(FAILED(hres))return hres;str[0] = (char)tmp;break;- }
You only need first character, right? Then why do you need a full BSTR pointer in VT_BSTR case? And assigning it to 'str' you leak a previously allocated buffer. Why cast to (char)tmp?
- else if(len == 0)
newstr = '\0';
Same way you're losing pointer to allocated buffer.
Thank you very much for commenting on this patch.
2014-10-20 0:06 GMT+08:00 Nikolay Sivov bunglehead@gmail.com:
- str = SysAllocStringLen(NULL, 1023);
- newstr = SysAllocStringLen(NULL, 1023);
Where this length comes from? Well, I tested String on windows xp, and found that 1023 was the limit, when given a number bigger than that, the output kept the length of 1023.
- switch(V_VT(arg + 1)) {
- case VT_NULL:
return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE);- case VT_BSTR:
str = V_BSTR(arg + 1);break;- case VT_ARRAY|VT_BYREF|VT_VARIANT:
return DISP_E_TYPEMISMATCH;- default:
hres = to_short(arg + 1, &tmp);if(FAILED(hres))return hres;str[0] = (char)tmp;break;- }
You only need first character, right? Then why do you need a full BSTR pointer in VT_BSTR case? And assigning it to 'str' you leak a previously allocated buffer.
So how do I get the first character of (arg + 1)? How about this: str[0] = * V_BSTR(arg + 1) In fact I don't quite understand how SysAllocStringLen work, but I see it is used in the former function, so I think maybe it is necessary.
Why cast to (char)tmp?
I think the type of str[0] is WCHAR, and tmp is an integer, shouldn't we make a cast?
- else if(len == 0)
newstr = '\0';Same way you're losing pointer to allocated buffer.
On 10/20/2014 19:58, Shuai Meng wrote:
Thank you very much for commenting on this patch.
2014-10-20 0:06 GMT+08:00 Nikolay Sivov <bunglehead@gmail.com mailto:bunglehead@gmail.com>:
+ str = SysAllocStringLen(NULL, 1023); + newstr = SysAllocStringLen(NULL, 1023); Where this length comes from? Well, I tested String on windows xp, and found that 1023 was the limit, when given a number bigger than that, the output kept the length of 1023.
Can you add this too as a test? In some compact way if possible.
+ switch(V_VT(arg + 1)) { + case VT_NULL: + return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE); + case VT_BSTR: + str = V_BSTR(arg + 1); + break; + case VT_ARRAY|VT_BYREF|VT_VARIANT: + return DISP_E_TYPEMISMATCH; + default: + hres = to_short(arg + 1, &tmp); + if(FAILED(hres)) + return hres; + str[0] = (char)tmp; + break; + } You only need first character, right? Then why do you need a full BSTR pointer in VT_BSTR case? And assigning it to 'str' you leak a previously allocated buffer.So how do I get the first character of (arg + 1)? How about this: str[0] = * V_BSTR(arg + 1) In fact I don't quite understand how SysAllocStringLen work, but I see it is used in the former function, so I think maybe it is necessary.
What I don't get is why do you allocate 'str' at all, it should be just 'WCHAR ch;' variable.
Why cast to (char)tmp?I think the type of str[0] is WCHAR, and tmp is an integer, shouldn't we make a cast?
WCHAR is 'unsigned short'. And it feels like it needs more tests for other variant types.
+ else if(len == 0) + newstr = '\0'; Same way you're losing pointer to allocated buffer.
Thanks for commenting.
2014-10-21 0:20 GMT+08:00 Nikolay Sivov bunglehead@gmail.com:
On 10/20/2014 19:58, Shuai Meng wrote:
Thank you very much for commenting on this patch.
2014-10-20 0:06 GMT+08:00 Nikolay Sivov bunglehead@gmail.com:
- str = SysAllocStringLen(NULL, 1023);
- newstr = SysAllocStringLen(NULL, 1023);
Where this length comes from? Well, I tested String on windows xp, and found that 1023 was the limit, when given a number bigger than that, the output kept the length of 1023.
Can you add this too as a test? In some compact way if possible.
I tried again and I found that maybe you were right. The number of 1023 was got like this: I wrote vbscript with a common text editor on xp, and save it as xx.vbs, then double clicked it. I used this method and found that MsgBox String(1024, 65) only printed "a...(the length is 1023)....a". Tragedy is today I write MsgBox Len(String(1024, 65)), and it prints 1024! It's the same with the other numbers greater than 1023! So I was cheated yesterday. The true limit may be 32768 * 16384 * 1.25， but I am not sure. I need to know the exact limit of the length of the string subtype, however experiments show difference with the documents of microsoft which claims the limit is about 2 billion.
- switch(V_VT(arg + 1)) {
- case VT_NULL:
return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE);- case VT_BSTR:
str = V_BSTR(arg + 1);break;- case VT_ARRAY|VT_BYREF|VT_VARIANT:
return DISP_E_TYPEMISMATCH;- default:
hres = to_short(arg + 1, &tmp);if(FAILED(hres))return hres;str[0] = (char)tmp;break;- }
You only need first character, right? Then why do you need a full BSTR pointer in VT_BSTR case? And assigning it to 'str' you leak a previously allocated buffer.
So how do I get the first character of (arg + 1)? How about this: str[0] = * V_BSTR(arg + 1) In fact I don't quite understand how SysAllocStringLen work, but I see it is used in the former function, so I think maybe it is necessary.
What I don't get is why do you allocate 'str' at all, it should be just 'WCHAR ch;' variable.
Yes, you are right.
Why cast to (char)tmp?
I think the type of str[0] is WCHAR, and tmp is an integer, shouldn't we make a cast?
WCHAR is 'unsigned short'. And it feels like it needs more tests for other variant types.
Yes, such as VT_DATE, but problem is it can't be tested. Until now, '#' is still not supported in wine.
- else if(len == 0)
newstr = '\0';Same way you're losing pointer to allocated buffer.
On 10/23/2014 21:07, Shuai Meng wrote:
Thanks for commenting.
2014-10-21 0:20 GMT+08:00 Nikolay Sivov <bunglehead@gmail.com mailto:bunglehead@gmail.com>:
On 10/20/2014 19:58, Shuai Meng wrote:Thank you very much for commenting on this patch. 2014-10-20 0:06 GMT+08:00 Nikolay Sivov <bunglehead@gmail.com <mailto:bunglehead@gmail.com>>: + str = SysAllocStringLen(NULL, 1023); + newstr = SysAllocStringLen(NULL, 1023); Where this length comes from? Well, I tested String on windows xp, and found that 1023 was the limit, when given a number bigger than that, the output kept the length of 1023.Can you add this too as a test? In some compact way if possible.I tried again and I found that maybe you were right. The number of 1023 was got like this: I wrote vbscript with a common text editor on xp, and save it as xx.vbs, then double clicked it. I used this method and found that MsgBox String(1024, 65) only printed "a...(the length is 1023)....a". Tragedy is today I write MsgBox Len(String(1024, 65)), and it prints 1024! It's the same with the other numbers greater than 1023! So I was cheated yesterday. The true limit may be 32768 * 16384 * 1.25， but I am not sure. I need to know the exact limit of the length of the string subtype, however experiments show difference with the documents of microsoft which claims the limit is about 2 billion.
In that case we don't care about exact limit if any. Just allocate what's requested and fail with proper error code if allocation failed.
-
Nikolay Sivov -
Shuai Meng