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

Re: WININET: take account of dwHeaderLength in HttpSendRequest

by Dmitry Timoshkov

"Mike McCormack" <mike(a)codeweavers.com> wrote: > Does this look better? Strange I didn't get a compiler warning from that... > + if( dwHeaderLength == -1L ) > + len = strlenW(lpszHeaders); > + else > + len = dwHeaderLength; Well, it's better, but really needs a test to verify whether only -1 or any negative value is treated as a hidden strlen. -- Dmitry.

Aug. 3, 2004

Re: WININET: take account of dwHeaderLength in HttpSendRequest

by Dmitry Timoshkov

"Mike McCormack" <mike(a)codeweavers.com> wrote: > ChangeLog: > * take account of dwHeaderLength in HttpSendRequest > - len = strlenW(lpszHeaders)+3; > - szCookedHeaders = HeapAlloc( GetProcessHeap(), 0, sizeof(WCHAR)*len ); > - strcpyW( szCookedHeaders, lpszHeaders ); > + if( dwHeaderLength < 0 ) > + len = strlenW(lpszHeaders); > + else > + len = dwHeaderLength; (dwHeaderLength < 0) is an always true condition, since dwHeaderLength is DWORD, i.e. unsigned long. -- Dmitry.

Aug. 3, 2004

Re: wine metacity and ctrl-alt-del binding - ctrl sticks

by Mike Hearn

This is a well known problem, you don't have to hit ctrl-alt-delete, for many users simply switching virtual desktops or using alt+tab will do it. Unfortunately it seems that Alexandres wm (fvwm?) does not exhibit this issue with Wine .... Grant Williamson wrote: > Curious if there is a patch somewhere for this. > > Running rhel3, gnome-2.2 with metacity. > > 1) Start notepad, type a few characters > 2) hit the keyboard combination ctrl-alt-del ... "gnome-session-kill > --save" is started. > 3) Cancel this option. > 4) Now try typing in Notepad, i.e. hit the keys "s" or "f", its just > like ctrl-s or ctrl-f have been held in. > 5) Hitting ctrl once cancels this. > > Anyone have a patch, to stop this happening? > > >

Aug. 3, 2004

wine metacity and ctrl-alt-del binding - ctrl sticks

by Grant Williamson

Curious if there is a patch somewhere for this. Running rhel3, gnome-2.2 with metacity. 1) Start notepad, type a few characters 2) hit the keyboard combination ctrl-alt-del ... "gnome-session-kill --save" is started. 3) Cancel this option. 4) Now try typing in Notepad, i.e. hit the keys "s" or "f", its just like ctrl-s or ctrl-f have been held in. 5) Hitting ctrl once cancels this. Anyone have a patch, to stop this happening?

Aug. 3, 2004

Re: list view completely mangled in WINE

by Tobias Burnus

Hello, Dimitrie O. Paun wrote: > 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? I used WINE (more precisely CrossOver Office 3.0.1 and CXO nightly) to install the program, which worked flawlessly. Using the CXO config file, it also installed with the WINE CVS version. Tobias

Aug. 3, 2004

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. 2, 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. 2, 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. 2, 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. 2, 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. 2, 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

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

by Jeroen Janssen

James Hawkins wrote: > 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? Yes, if you have a char (1byte) pointer and you ++, you'll get one byte further. If you have a short (2byte) you'll get two bytes further in memory. If you have a long (4byte) pointer, you'll get 4 bytes further in memory if you ++. It is simular to dealing with arrays (looking at the memory layout) If you have an array of structs and need to get to the next struct in the array then you will need to look sizeof(struct) further in memory to find the next item in the array. --- Jeroen

Aug. 2, 2004

Re: Dynamic memory management in CryptAcquireContext

by Alexandre Julliard

Michael Jung <mjung(a)iss.tu-darmstadt.de> writes: > If the CSP's CPAcquireContext function fails, which is quite common (e.g. > because the user specified key container does not exist), the "temp" variable > is still free'd twice. Yes, the free needs to be moved inside the if, sorry about that. > I'm aware that my original patch was pretty ugly, due to a lot of redundancy, > but I think this is due to fact that the CryptAcquireContext function seems > not to be very well structured. I've submitted a patch two weeks ago, which > basically was a reimplementation of CryptAcquireContext, but which was not > committed (http://www.winehq.org/hypermail/wine-patches/2004/07/0213.html) > Rob Shearman commented on this patch and felt that it is overkill to add both > a helper function and a new struct type. Is this the reason the patch was not > applied? It's overkill and it's ugly. If a function needs such elaborate cleanup it's probably because it's trying to do too many things, and that needs to be fixed; splitting the cleanup part into a separate function only makes things even more confusing. -- Alexandre Julliard julliard(a)winehq.org

Aug. 2, 2004

Re: list view completely mangled in WINE

by Filip Navara

Tobias Burnus wrote: > The patch improves the situation a lot! Even Save List works now! > Thanks! Great! [snip] > Now, I click on the scrollbar to scroll to the rightmost columns; > while the header is correct, the list itself contains the FIRST > columns not the columns which belong to the column headers > http://www.physik.fu-berlin.de/~tburnus/wine/diamond_list_wine3b.png Should be fixed in the attached patch, but there is still one problem with refreshing of the list view after shrinking the column headers. Unfortunetly I don't have time to fix that. Regards, Filip

Aug. 2, 2004

Re: RESEND: Try to avoid loading 16-bit DLLs into memory for accessing version info

by Alexandre Julliard

"Dmitry Timoshkov" <dmitry(a)baikal.ru> writes: > "Alexandre Julliard" <julliard(a)winehq.org> wrote: >> I think it would >> be better to try to take loadorder into account, though I admit it's >> not trivial to do. > > Any idea how to do it without breaking DLL separation? A possibility would be to use wine_dll_get_owner; if the dll doesn't have an owner then you know it's native only and you can access the file directly. This way you only do the LoadLibrary16 for dlls that have a builtin implementation, it's not perfect but probably good enough. -- Alexandre Julliard julliard(a)winehq.org

Aug. 2, 2004

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

by James Hawkins

> - 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. On Mon, 02 Aug 2004 19:55:12 +0900, Mike McCormack <mike(a)codeweavers.com> wrote: > > James Hawkins wrote: > > > + PWSTR keyname; > > > > - keyname = CRYPT_Alloc(strlen(KEYSTR) + strlen(pProvName) +1); > > + keyname = CRYPT_Alloc((strlenW(KEYSTR) + strlenW(pProvName) + 1) * sizeof(WCHAR)); > > if (keyname) > > { > > - strcpy(keyname, KEYSTR); > > - strcpy(keyname + strlen(KEYSTR), pProvName); > > + strcpyW(keyname, KEYSTR); > > + strcpyW(keyname + strlenW(KEYSTR) * sizeof(WCHAR), pProvName); > > } else > > SetLastError(ERROR_NOT_ENOUGH_MEMORY); > > return keyname; > > } > > This doesn't look right.... when using pointer arithmetic, you do so in > increments of the pointer's type (eg. WCHAR). The following two lines > are the same, and are both wrong: > > strcpyW(keyname + strlenW(KEYSTR) * sizeof(WCHAR), pProvName); > strcpyW(&keyname[strlenW(KEYSTR) * sizeof(WCHAR)], pProvName); > > The right version would be: > > strcpyW(&keyname[strlenW(KEYSTR)], pProvName); > > but how about just using: > > strcatW( keyname, pProvname ); > > Mike > -- James Hawkins

Aug. 2, 2004

Re: Anyone writing COM tests?

by Duane Clark

Jeroen Janssen wrote: > > I think writing (COM) tests is a great idea. > I have 'hit' a few bumps in the road of wine & COM in the past few weeks > and I would appreciate it if I can help with writing (specific) > testscenarios. > > It would be great to have a small 'com test framework' & sample code to > base specific tests upon. I'm wondering though if all can be build > without problems : I'm used to VC6, but it would be better if things can > be (cross) build with mingw and/or gcc + with winelib. I would be using mingw and gcc, since I don't have VC. It would certainly help to have someone else compiling on VC to make sure the code works there. > > Also generating proxy/stub code (without using midl) might be a problem > at this point. I might try to getting an old copy of VC to get midl. It might be useful for tests comparing results with widl.

Aug. 2, 2004

Re: Anyone writing COM tests?

by Jeroen Janssen

Duane Clark wrote: > Howdy, > > Debugging a problem I am having in COM (mentioned a few weeks ago), I > think I know what the problem is but not where. So I figured some COM > test would help me to understand how things should work. I was a bit > surprised to see no COM tests, or at least I didn't find any in the ole* > directories. So I figured if no one else is working on them, I might > take a shot at it over the next few weeks/months. And maybe this would > encourage some updates to the COM implementation. I think writing (COM) tests is a great idea. I have 'hit' a few bumps in the road of wine & COM in the past few weeks and I would appreciate it if I can help with writing (specific) testscenarios. It would be great to have a small 'com test framework' & sample code to base specific tests upon. I'm wondering though if all can be build without problems : I'm used to VC6, but it would be better if things can be (cross) build with mingw and/or gcc + with winelib. Also generating proxy/stub code (without using midl) might be a problem at this point. --- Jeroen Janssen

Aug. 2, 2004

Anyone writing COM tests?

by Duane Clark

Howdy, Debugging a problem I am having in COM (mentioned a few weeks ago), I think I know what the problem is but not where. So I figured some COM test would help me to understand how things should work. I was a bit surprised to see no COM tests, or at least I didn't find any in the ole* directories. So I figured if no one else is working on them, I might take a shot at it over the next few weeks/months. And maybe this would encourage some updates to the COM implementation.

Aug. 2, 2004

Winrash / Winetest

by Stephen Ogletree

(I sent a version of this to wine-users originally and Ivan said to send it over here to wine-devel) On windows XP, with the latest patches/service packs, I got the errors below. Since these were popups and/or crashes, I'm assuming they need to be reported manually. 0. Small issue: The winrash installer (NSIS) concludes with a window that has 3 lines that are in blue text to suggest a hyperlink (and the surrounding text even says "Click the text below to make a bookmark for the test results") but clicking the text has no visible effect (when moving the cursor over the text, the cursor does change shape to indicate the text is clickable, so at least that much works...). My default browser is Opera and my security settings in IE are maxed out, with everything possible disabled. In Opera, I typically keep popups, plugins, and javascript disabled, but none of that should keep normal URL to a html page from opening. Since installing winrash didn't do anything except put the service in place and start it, I figured I'd go ahead and download the test manually. This rest of this email pertains to /winetest-200407091000-kevin-mingw.exe/ 1. I received the "Can't parse subtests output of msvcrtd" message during 'extraction'. I assume this is the same error Holly reported. 2. I had a message indicating a timeout during the user32 tests (before the network tests - this appears to be the same message Holly reported). Also, I remember the timeout was occurring back when we first tried out this testing program a month or two (or 3?) ago, so its a bug thats been around a while. 3. I am still seeing an error that I pointed out in the first round (or a long time ago, at least..) of testing where the test crashes Explorer (this is after the timeout, but before the prompt to send the results). Explorer crashing causes all of my desktop icons to reset to their positions from when I last logged in/out (which has been a week or two, so this was significant change), as well as causes some of the SysTray icons to vanish, and finally, of course I lose the start menu bar and the bar containing the running application names for a moment, while Windows XP nicely restarts explorer.exe. Meanwhile, there's no indication of trouble from the winetest window. FWIW: When I agreed to submit my test results, I did *not* receive any warning about my log being too big. Stephen Ogletree

Aug. 2, 2004

Dynamic memory management in CryptAcquireContext

by Michael Jung

Alexandre, I've noticed that you applied a modified version of my patch, which tries to fix a potential double free of the "temp" variable in dlls/advapi32/crypt.c's CryptAcquireContextA function. However, I think that the modified version does not fix the problem. Here is a code snippet from the current cvs versionc of crypt.c: CRYPT_Free(temp); ... if (pProv->pFuncs->pCPAcquireContext(&pProv->hPrivate, (CHAR*)pszContainer, dwFlags, pProv->pVTable)) { ... return TRUE; } /* FALLTHROUGH TO ERROR IF FALSE - CSP internal error! */ error: ... CRYPT_Free(temp); ... return FALSE; If the CSP's CPAcquireContext function fails, which is quite common (e.g. because the user specified key container does not exist), the "temp" variable is still free'd twice. I'm aware that my original patch was pretty ugly, due to a lot of redundancy, but I think this is due to fact that the CryptAcquireContext function seems not to be very well structured. I've submitted a patch two weeks ago, which basically was a reimplementation of CryptAcquireContext, but which was not committed (http://www.winehq.org/hypermail/wine-patches/2004/07/0213.html) Rob Shearman commented on this patch and felt that it is overkill to add both a helper function and a new struct type. Is this the reason the patch was not applied? I would be glad to rework the patch, if you give me some hints about what you did not like. However, I would prefer to wait until James Hawkins got his a->w cross call cleanup patch committed, in order not to break his work. Greetings, Michael -- Michael Jung, MSc Be liberal in what you accept, Integrated Circuits and Systems Lab and conservative in what you send. University of Technology, Darmstadt -- Jonathan Postel, Internet Pioneer phone: +49(6151)16-6692 / email: mjung(a)iss.tu-darmstadt.de

Aug. 2, 2004