August 2004 - Wine-Devel - list.winehq.org

Re: list view completely mangled in WINE

by Robert Shearman

Dimitrie O. Paun wrote: >On Tue, Aug 03, 2004 at 01:42:28AM +0200, Filip Navara wrote: > > >>have inserted debug messages on all the places where the listview >>display mode can be changed and none of them appeared. After going once >>more through the whole code I found this. Obviously it seemed redundant >>and the only reason for doing that was workarounding problems with >>changed WS_VSCROLL and WS_HSCROLL styles (which correctly don't recieve >>WM_STYLECHANGED notifications from the scrolling code). So I tried >>removing it and ... whoa ... to my surprise it started to work. >> >> > >The problem is that WM_STYLECHANGED is sometimes sent, sometimes not, >and it seems so much easier to just update it every time we come in >so we can be sure we're using the latest value, as we should. > The toolbar control works fine just using WM_STYLECHANGED to notify when it needs to update its internal state and it seems to send the message for all style changes that it needs (it doesn't depend on WS_ styles). >It's >redundant, I agree, but it's neglijable in terms of performance so >I've put it in due to it's simplicity. This way the dwStyle has a >very simple to understand semantics. In fact, I've meant to remove >it eventually, just like you did, but only as an optimization. However, >doing so to fix a bug in a way that we don't understand worries me >greatly. I'd rather keep it in, and fix the problem. > Rob

Aug. 3, 2004

Re: cleanup a->w cross calls in advapi32/crypt.c

by James Hawkins

I have compiled crypt and run the tests with no failures. This latest patch removes all changes to the pointer arithmetic. On Tue, 03 Aug 2004 07:55:51 +0300, Shachar Shemesh <wine-devel(a)shemesh.biz> wrote: > James Hawkins wrote: > > >>- strcpy(keyname + strlen(KEYSTR), pProvName); > >> > >> > >I changed that line to strcatW(keyname, pProvName). That makes a lot > >more sense Mike, thanks for the tip. > > > >I'm wasn't exactly sure on this one so it would be great if you could > >help me on this one. When using pointer arithmetic, do the operations > >such as --, ++... increment or decrement by the size of the pointer > >type? For example, > > > >*(ptr - sizeof(WCHAR)) = (dwType % 10) + '0'; > >*(ptr - sizeof(WCHAR) * 2) = ((dwType / 10) % 10) + '0'; > >*(ptr - sizeof(WCHAR) * 3) = (dwType / 100) + '0'; > > > >Is the sizeof(WCHAR) multiplication redundant because --ptr actually > >moves ptr down one WCHAR? I understand that if that's the case, but > >what about when the pointer is first initialized? > > > >ptr = keyname + strlenW(keyname); > > > >I guess it holds true here as well because keyname is a pointer and > >we're adding ot it. If that is the case, the included patch fixes the > >two things mentioned. > > > > > Ok, whenever such matters come up, it's always wise to go to the source > and ask the most authorative book there is. This is what the bible has > to say about this (bible = "The C Programing Language, 2nd edition"): > Page 98 bottom: > "These remarks are true regardless of the type or size of the variables > in the array a. The meaning of "adding 1 to a pointer," and by > extension, all pointer arithmetics, is that pa+1 points to the next > object, and pa+i points to the i-th object beyond pa." > > In other words, ANY time you add or subtract and integer from a pointer, > the result is to move the pointer to another array element. No need to > perform these "word size" calculations anywhere. > > Question: Did you compile and check you patch? > > Shachar > > -- > Shachar Shemesh > Lingnu Open Source Consulting ltd. > http://www.lingnu.com/ > > -- James Hawkins

Aug. 3, 2004

Re: cleanup a->w cross calls in advapi32/crypt.c

by Shachar Shemesh

James Hawkins wrote: >>- strcpy(keyname + strlen(KEYSTR), pProvName); >> >> >I changed that line to strcatW(keyname, pProvName). That makes a lot >more sense Mike, thanks for the tip. > >I'm wasn't exactly sure on this one so it would be great if you could >help me on this one. When using pointer arithmetic, do the operations >such as --, ++... increment or decrement by the size of the pointer >type? For example, > >*(ptr - sizeof(WCHAR)) = (dwType % 10) + '0'; >*(ptr - sizeof(WCHAR) * 2) = ((dwType / 10) % 10) + '0'; >*(ptr - sizeof(WCHAR) * 3) = (dwType / 100) + '0'; > >Is the sizeof(WCHAR) multiplication redundant because --ptr actually >moves ptr down one WCHAR? I understand that if that's the case, but >what about when the pointer is first initialized? > >ptr = keyname + strlenW(keyname); > >I guess it holds true here as well because keyname is a pointer and >we're adding ot it. If that is the case, the included patch fixes the >two things mentioned. > > Ok, whenever such matters come up, it's always wise to go to the source and ask the most authorative book there is. This is what the bible has to say about this (bible = "The C Programing Language, 2nd edition"): Page 98 bottom: "These remarks are true regardless of the type or size of the variables in the array a. The meaning of "adding 1 to a pointer," and by extension, all pointer arithmetics, is that pa+1 points to the next object, and pa+i points to the i-th object beyond pa." In other words, ANY time you add or subtract and integer from a pointer, the result is to move the pointer to another array element. No need to perform these "word size" calculations anywhere. Question: Did you compile and check you patch? Shachar -- Shachar Shemesh Lingnu Open Source Consulting ltd. http://www.lingnu.com/

Aug. 3, 2004

Re: cleanup a->w cross calls in advapi32/crypt.c

by Dmitry Timoshkov

"James Hawkins" <truiken(a)gmail.com> wrote: > ptr = keyname + strlenW(keyname); > > I guess it holds true here as well because keyname is a pointer and > we're adding ot it. If that is the case, the included patch fixes the > two things mentioned. You have to look for other similar, not mentioned cases, and fix them as well. -- Dmitry.

Aug. 3, 2004

Re: list view completely mangled in WINE

by Dimitrie O. Paun

On Tue, Aug 03, 2004 at 03:17:27AM +0200, Filip Navara wrote: > I did another test. I intserted a code into the ListView window > procedure to print a message every time the internal dwStyle member > doesn't match the one returned by GetWindowLong with GWL_STYLE. To my > little surprise they were different right from the beginning (even for > WM_CREATE message). The one returned by GetWindowLong didn't contain the > listview specific flags, If so, how did the listview work at all until now?!? > WS_VSCROLL/WS_HSCROLL sometimes didn't match > (as I explained why) and for the WS_CREATE message the WS_VISIBLE flag > was different. I searched my local MSDN copy in order to find why this > happens and the only relevant article I found is this: > http://support.microsoft.com/default.aspx?scid=kb;EN-US;Q83366. The Good stuff, maybe we should link to it from WineHQ > "SUMMARY" part of the article sort of explains why this happens (at > least according to my understanding) and so I think the patch is OK. Do > you agree? Almost. If it does go in though, we need to: -- fix a few more places where we look at WS_[VH]SCROLL -- understand why did the listview work at all if the LVS_ stuff was missing -- do an audit (or add a Janitorial task) to audit and fix the other controls as well. (of course, only the 1st point is really needed for the patch to go in, but the other two would be nice to have). -- Dimi.

Aug. 3, 2004

Re: list view completely mangled in WINE

by Filip Navara

Dimitrie O. Paun wrote: [snip] >The problem is that WM_STYLECHANGED is sometimes sent, sometimes not, >and it seems so much easier to just update it every time we come in >so we can be sure we're using the latest value, as we should. It's >redundant, I agree, but it's neglijable in terms of performance so >I've put it in due to it's simplicity. This way the dwStyle has a >very simple to understand semantics. In fact, I've meant to remove >it eventually, just like you did, but only as an optimization. However, >doing so to fix a bug in a way that we don't understand worries me >greatly. I'd rather keep it in, and fix the problem. > I did another test. I intserted a code into the ListView window procedure to print a message every time the internal dwStyle member doesn't match the one returned by GetWindowLong with GWL_STYLE. To my little surprise they were different right from the beginning (even for WM_CREATE message). The one returned by GetWindowLong didn't contain the listview specific flags, WS_VSCROLL/WS_HSCROLL sometimes didn't match (as I explained why) and for the WS_CREATE message the WS_VISIBLE flag was different. I searched my local MSDN copy in order to find why this happens and the only relevant article I found is this: http://support.microsoft.com/default.aspx?scid=kb;EN-US;Q83366. The "SUMMARY" part of the article sort of explains why this happens (at least according to my understanding) and so I think the patch is OK. Do you agree? - Filip

Aug. 3, 2004

Re: list view completely mangled in WINE

by Filip Navara

Dimitrie O. Paun wrote: > On Tue, Aug 03, 2004 at 01:42:28AM +0200, Filip Navara wrote: > >> Unfortunetly I can't explain it, because I don't understand it >> myself. I > > In which case I must object to the patch :( It seems we're just hiding > a problem in a very convoluted way, don't you think? Well, I agree, the patch shouldn't go in, but I don't have currently time to investigate the issue behind this. Maybe in a week or so. If you want to give it a try, feel free to do so... - Filip

Aug. 3, 2004

Re: list view completely mangled in WINE

by Dimitrie O. Paun

On Tue, Aug 03, 2004 at 01:42:28AM +0200, Filip Navara wrote: > Unfortunetly I can't explain it, because I don't understand it myself. I In which case I must object to the patch :( It seems we're just hiding a problem in a very convoluted way, don't you think? > have inserted debug messages on all the places where the listview > display mode can be changed and none of them appeared. After going once > more through the whole code I found this. Obviously it seemed redundant > and the only reason for doing that was workarounding problems with > changed WS_VSCROLL and WS_HSCROLL styles (which correctly don't recieve > WM_STYLECHANGED notifications from the scrolling code). So I tried > removing it and ... whoa ... to my surprise it started to work. The problem is that WM_STYLECHANGED is sometimes sent, sometimes not, and it seems so much easier to just update it every time we come in so we can be sure we're using the latest value, as we should. It's redundant, I agree, but it's neglijable in terms of performance so I've put it in due to it's simplicity. This way the dwStyle has a very simple to understand semantics. In fact, I've meant to remove it eventually, just like you did, but only as an optimization. However, doing so to fix a bug in a way that we don't understand worries me greatly. I'd rather keep it in, and fix the problem. -- Dimi.

Aug. 2, 2004

Re: list view completely mangled in WINE

by Filip Navara

Dimitrie O. Paun wrote: >Can you please explain why updating it is a problem? Yes, we may be >doing more work than needed, but at most it should be a no-op. >In fact, we used to call 'GetWindowLongW(hwnd, GWL_STYLE)' every >time we needed to get to the dwStyle, so updating it in there >should be OK. > > Unfortunetly I can't explain it, because I don't understand it myself. I have inserted debug messages on all the places where the listview display mode can be changed and none of them appeared. After going once more through the whole code I found this. Obviously it seemed redundant and the only reason for doing that was workarounding problems with changed WS_VSCROLL and WS_HSCROLL styles (which correctly don't recieve WM_STYLECHANGED notifications from the scrolling code). So I tried removing it and ... whoa ... to my surprise it started to work. >BTW, I've tried to install the demo, but the installer dies on my >version of wine. Does it work for you, or did you use Windows for >the installation? > Yes, I installed it on Windows and even most of done most of the testing on Windows with Wine COMCTL32. Before sending the patch to wine-patches I checked if it works as desired on Wine... - Filip

Aug. 2, 2004

Re: list view completely mangled in WINE

by Dimitrie O. Paun

On Sat, Jul 31, 2004 at 05:52:21AM +0200, Filip Navara wrote: > Tobias Burnus wrote: > [snip] > > Can you try the attached patch please? > > Changelog: > - Don't update infoPtr->dwStyle in LISTVIEW_WindoProc. It's already > handled in LISTVIEW_StyleChanged and LISTVIEW_Create processing. > > - if (infoPtr) > - { > - infoPtr->dwStyle = GetWindowLongW(hwnd, GWL_STYLE); > - } > - > switch (uMsg) > { > case LVM_APPROXIMATEVIEWRECT: Can you please explain why updating it is a problem? Yes, we may be doing more work than needed, but at most it should be a no-op. In fact, we used to call 'GetWindowLongW(hwnd, GWL_STYLE)' every time we needed to get to the dwStyle, so updating it in there should be OK. BTW, I've tried to install the demo, but the installer dies on my version of wine. Does it work for you, or did you use Windows for the installation? -- Dimi.

Aug. 2, 2004