From: Jeremy W. <jez...@ho...> - 2005-10-17 08:02:43
|
>>I've just committed a fix for Combobox.xs (GetString) and Listbox.xs >>(GetString) - both weren't allocating enough space for > >Do you think this problem would have been more likely to show up on Windows >XP than on Windows 2000? I have an application that seems quite robust on >Windows 2000, but it has had a mysterious crash on Windows XP. Combobox >manipulations have been suspected from the stack traces, but I've never >been able to track it down. If you are using Perl 5.8.x, then yes this could be the cause of your problem. I'm using XP. I'll give you an example of how this issue was effecting me, to show you how odd the effects could be. My application ran fine under 5.6.1, but as soon as it was running under 5.8.7, I was getting the strangest crashes. At first, I suspected my own XS modules, so I spent several hours separating the GUI from the logic of the application. Still crashes. ARH! I tracked it down to a single perl substr - comment out the substr - no crash - put a print statement before the substr - no crash. The stack trace showed that perl was actually crashing at the substr, but it was crashing via free, and finally going belly up in one of the core MS dll's. Clearly, this was a memory issue, but I didn't know where. Stripping down the application even more, I came across a different crash where passing 'undef' to the tree insert item caused a crash. In the logic of my app, I was updating various drop downs as items are added to this tree. This turned out to be the key, as once I got around the tree crash, I was getting the first stack traces where the crash was occurring in SendMessage (Combobox.xs, just after the problem safemalloc). It was still occurring at random times, but at least I has some sort of 'provable' case. The issue is simply that we were overflowing the buffer by 1 character - in this case by writing zero ("\0"). Now, in pervious versions of perl (or indeed, depending on the underlying OS) the actual pointer returned by safemalloc might be larger than the buffer requested. In these cases, there would never be an issue. Even if the buffer returned by safemalloc was exactly the requested size, it might not cause a crash or even a problem, depending on what the underlying application is doing. In short, almost random crashes! >Testing will take a few days, because I need to ship a new version to my >client that encounters the problem regularly... I can't produce it on >demand like she can, although I have produced it a few times on XP, but >never on 2K. I would be interested to hear if this problem goes away. I still suspect there are still a few problems with Win32-GUI and 5.8.x. >>All solutions aren't Unicode aware, and going forward it's probably a good >>idea to create a function/macro to handle the allocation of strings. > >Yeah, it'd be nice to make Win32::GUI more Unicode aware. That's something >that I'm really interested in, and would be willing to help figure out and >work on, although to date I've not had much success in the figuring out >part (Rob has helped me get enough Unicode into one application to make me >happy, and that is my highest priority app for the present, to get that >working more completely...) http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/platform/commctls/comboboxes/comboboxreference/comboboxmessages/cb_getlbtextlen.asp Have a quick look at the above link. Basically, in all the cases where we are using safemalloc to create a buffer large enough to hold a string, we're using a MS function which returns the number of characters in the item. Instead of assuming bytes, we would assume Unicode and the resulting buffer would always be large enough. We really should create a helper function that allocates enough space and use that instead of manually doing malloc. Something like: LPTSTR safemallocstring(int size) { size++; #add one to ensure we have enough space for the terminating char return (LPCTSTR) safemalloc (size*2); } Cheers, jez. |
From: Jeremy W. <jez...@ho...> - 2005-10-18 09:40:07
|
>Seems like that should suffice, assuming that MSDN's definition of >"character" is a 32-bit UCS-2 character. I have no clue if MS supports >UCS-2 characters that use multiple 32-bit numbers to represent a single >character, but I believe such are legal. And if they do support them, I >don't know if they call them one character or two, or if they are even >consistent about their counts, or their definition of "character" ("That >depends on what you mean by 'is'.") That's a good point - more research would be needed. At least with one function doing all the string memory allocation we've only got one place to change. Cheers, Jez. |
From: Robert M. <rm...@po...> - 2005-10-18 18:36:12
|
Jeremy White wrote: >> Seems like that should suffice, assuming that MSDN's definition of >> "character" is a 32-bit UCS-2 character. I have no clue if MS >> supports UCS-2 characters that use multiple 32-bit numbers to >> represent a single character, but I believe such are legal. And if >> they do support them, I don't know if they call them one character or >> two, or if they are even consistent about their counts, or their >> definition of "character" ("That depends on what you mean by 'is'.") > > That's a good point - more research would be needed. At least with one > function doing all the string memory allocation we've only got one place > to change. I haven't really checked this out fully, but looked into some of the quite recently (I'm still thinking in the background about what we would need to do to get a Unicode version of Win32::GUI). The return value is the number of TCHARs required, where a TCHAR is one byte (char) or 2 bytes (w_char?) depending on the setting of the _UNICODE pre-processor symbol at compile time, so the right way to do this is: safemalloc((num_bytes_required + 1) * sizeof(TCHAR)); I don't believe that we need to worry about the 4-byte representations (surrogate pairs), as these are accounted for in the return value. I think, that there's a better implementation though; what we seem to currenlty have is: (1) safealloc some bytes (2) get SendMessage to copy the text into those byte (3) return a pointer (which is then converted by xsubpp (via typemap) into code that creates a mortalised PV, which in turn must do a malloc and copy) (4) free the bytes we safealloc'd It would save a malloc, copy and free if we created the mortalised PV at the right size ourselves, passed the string pointer to SendMessage() and returned the PV. This would lead to a more complex macro/function, but would, I think be worth it. I'm not sure I got all this right, but I think you'll get the idea (there may be a better way to create the SV and grow it than I've shown here): void GetLBText(handle,index) HWND handle WPARAM index PREINIT: STRLEN cbString; SV * text; CODE: cbString = SendMessage(handle, CB_GETLBTEXTLEN, index, 0); if(cbString != LB_ERR) { text = newSVpvn((char *)NULL, 0); if(SendMessage(handle, CB_GETLBTEXT, index, (LPARAM) SvGROW(text, cdString+1) ) != LB_ERR) { PUSHs(text); XSRETURN(1); } else { XSRETURN_UNDEF; } } else { XSRETURN_UNDEF; } This works nicely for the ANSI versions of the call, which return th ANSI encoded string of bytes that is suitable for directly inserting into the PV, but this doesn't address the Unicode issue at all, where the string copied by CB_GETLBTEXT would have to be converted from UCS-2 to utf-8 and then copied into the SV, setting the SVf_utf8 flag appropriately. Regards, Rob. |
From: Jeremy W. <jez...@ho...> - 2005-10-20 08:51:43
|
>This would lead to a more complex macro/function, but would, I think be >worth it. [snip] It would be a more complicated macro/function - but I agree with you, it would be worth it. >This works nicely for the ANSI versions of the call, which return th ANSI >encoded string of bytes that is suitable for directly inserting into the >PV, but this doesn't address the Unicode issue at all, where the string >copied by CB_GETLBTEXT would have to be converted from UCS-2 to utf-8 and >then copied into the SV, setting the SVf_utf8 flag appropriately. If we spend time on reworking some of the internals for Unicode is it worth us considering the implications of 64bit further down the road? Cheers, jez. |
From: Robert M. <rm...@po...> - 2005-10-20 17:18:54
|
Jeremy White wrote: > > If we spend time on reworking some of the internals for Unicode is it > worth us considering the implications of 64bit further down the road? I'd be very interested in starting to try to understand what 64bit support would entail. I'm starting to put together a list of things that don't work well with the current code-base, so that I can start to evaluate which bits of it might need significant re-work. Any pointer to what implications 64bit might have would be appreciated. Regards, Rob. |
From: Jeremy W. <jez...@ho...> - 2005-10-27 08:44:23
|
>>I'll let you know. > >Well, that fix appears to be the likeliest of the fixes that cured my >client's problem. She is delighted. And therefore, so am I. And this >confirms that the problem shows up more on XP than on Windows 2000, because >I could never reproduce it on Windows 2000, but she could always produce it >readily on XP, within an hour or so. Good - touch wood, I've had no issue either. Now we just need to sort out the strange issues that sometime occur on exit:) Cheers, jez. |