October 2008 - Wine-Devel - list.winehq.org

Re: (try 2)version: check for out of memory when VerInstallFileA/W converstion. (response to coverity 635)

by Aric Stewart

Whoops on the whitespace. The paths are irrelevent. They are intentionally pointing at a file and location that is not existing. Should I change the paths to express that more explicitly; something like: "D:\\oes\\not\\exist" ? -aric Rob Shearman wrote: > 2008/10/2 Aric Stewart <aric(a)codeweavers.com>: >> @@ -543,34 +543,50 @@ DWORD WINAPI VerInstallFileW( >> len = WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, NULL, 0, NULL, NULL ); >> if ((wsrcf = HeapAlloc( GetProcessHeap(), 0, len ))) >> WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, wsrcf, len, NULL, NULL ); >> + else >> + ret = VIF_OUTOFMEMORY; >> } >> - if (srcdir) >> + if (srcdir && !ret) > > Remove the extra space here. > >> + static const CHAR szSrcDir[] = "C:\\windows\\temp"; >> + static const CHAR szDestDir[] = "C:\\windows\\temp\\test"; >> + static const CHAR szCurDir[] = "C:\\"; > > This isn't exactly what I meant. C:\Windows might not even exist on > NT4/Win2000 and the temp directory might not be in %WINDIR%\temp. You > should use GetTempPath instead. (Hard-coding C:\ is also something to > be avoided if at all possible.) >

Oct. 2, 2008

Re: (try 2)version: check for out of memory when VerInstallFileA/W converstion. (response to coverity 635)

by Rob Shearman

2008/10/2 Aric Stewart <aric(a)codeweavers.com>: > @@ -543,34 +543,50 @@ DWORD WINAPI VerInstallFileW( > len = WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, NULL, 0, NULL, NULL ); > if ((wsrcf = HeapAlloc( GetProcessHeap(), 0, len ))) > WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, wsrcf, len, NULL, NULL ); > + else > + ret = VIF_OUTOFMEMORY; > } > - if (srcdir) > + if (srcdir && !ret) Remove the extra space here. > + static const CHAR szSrcDir[] = "C:\\windows\\temp"; > + static const CHAR szDestDir[] = "C:\\windows\\temp\\test"; > + static const CHAR szCurDir[] = "C:\\"; This isn't exactly what I meant. C:\Windows might not even exist on NT4/Win2000 and the temp directory might not be in %WINDIR%\temp. You should use GetTempPath instead. (Hard-coding C:\ is also something to be avoided if at all possible.) -- Rob Shearman

Oct. 2, 2008

Re: gdi32: if a font substitution is given then only match to that substitution not to the original family name.

by Dmitry Timoshkov

Aric Stewart <aric(a)codeweavers.com> wrote: > LIST_FOR_EACH(family_elem_ptr, &font_list) { > family = LIST_ENTRY(family_elem_ptr, Family, entry); > - if (!strcmpiW(family->FamilyName, FaceName) || > - (psub && !strcmpiW(family->FamilyName, psub->to.name))) > + if ((!psub && !strcmpiW(family->FamilyName, FaceName)) || > + ( psub && !strcmpiW(family->FamilyName, psub->to.name))) > { > LIST_FOR_EACH(face_elem_ptr, &family->faces) { > face = LIST_ENTRY(face_elem_ptr, Face, entry); Changing the order of 2 comparisons in the 'if' block would be clear IMO, and would help avoiding to check psub twice. -- Dmitry.

Oct. 2, 2008

Re: [PATCH 1/2] wininet: Return error codes explicitly from URLCache functions instead of returning a BOOL and storing the error code in the last error value.

by Juan Lang

Hi Rob, + * Locks the index for system-wide exclusive access. + * + * RETURNS + * TRUE if successful + * FALSE if failed and calls SetLastError. */ static LPURLCACHE_HEADER URLCacheContainer_LockIndex(URLCACHECONTAINER * pContainer) The comment's off here. @@ -1346,34 +1353,32 @@ static BOOL URLCache_AddEntryToHash(LPURLCACHE_HEADER pHeader, LPCSTR lpszUrl, D * FALSE if the hash table could not be created * */ -static HASH_CACHEFILE_ENTRY *URLCache_CreateHashTable(LPURLCACHE_HEADER pHeader, HASH_CACHEFILE_ENTRY *pPrevHash) +static DWORD URLCache_CreateHashTable(LPURLCACHE_HEADER pHeader, HASH_CACHEFILE_ENTRY *pPrevHash, HASH_CACHEFILE_ENTRY **ppHash) I think you forgot to update the comment here. --Juan

Oct. 2, 2008

Re: Fullscreen window in metacity hides owned window

by Dmitry Timoshkov

Michael Ost <most(a)museresearch.com> wrote: > We are seeing a problem in Metacity where a fullscreen window obscures a > second created window that has a particular style: WS_DLGFRAME | > WS_THICKFRAME. > > When window A is created fullscreen (WS_POPUP, size matches screen res) > and window B is created as an "owned window" > (WS_DLGFRAME|WS_THICKFRAME|WS_OVERLAPPED style with "A" as its parent) > you cannot see "B" if Metacity is running. > > You can if other window managers are running. We tried kwin, xfce, > fluxbox, enlightenment or fvvm. So the problem is something specific to > Metacity. > > You can also see window "B" if you remove either the WS_DLGFRAME or > WS_THICKFRAME bit from the style. > > Any hints about where to look within Wine or what could be happening? Have a look at dlls/winex11.drv/window.c. It looks like windows of type _NET_WM_WINDOW_TYPE_DIALOG have a similar problem to the one related to _NET_WM_WINDOW_TYPE_UTILITY. Try to comment out appropriate line of code and see if that helps. -- Dmitry.

Oct. 2, 2008

Re: version: check for out of memory when VerInstallFileA/W converstion. (response to coverity 635)

by Rob Shearman

2008/10/2 Aric Stewart <aric(a)codeweavers.com>: > @@ -543,34 +543,50 @@ DWORD WINAPI VerInstallFileW( > len = WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, NULL, 0, NULL, NULL ); > if ((wsrcf = HeapAlloc( GetProcessHeap(), 0, len ))) > WideCharToMultiByte( CP_ACP, 0, srcfilename, -1, wsrcf, len, NULL, NULL ); > + else if (len) > + ret = VIF_OUTOFMEMORY; IIRC HeapAlloc(..., 0) returns a non-NULL value, so the "if (len)" part is not needed. ... > + rc = VerInstallFileA(0x0, "in.txt", "out.txt", "C:\\windows\\temp", "C:\\windows\\temp\\test\\", "C:\\", tmpname, &size); > + ok (rc == 0x10000 && tmpname[0]==0," expected return 0x10000 and no tempname, got %08x/\'%s\'\n",rc,tmpname); You shouldn't hard-code paths such as this. -- Rob Shearman

Oct. 2, 2008

Re: Wine PulseAudio Driver

by Juan Lang

Hi Art, > Currently I am looking for any feedback at all. I believe you've already gotten some: split up your patches so that we can read them a little more easily. If you send a large patch as a new contributor, it's very unlikely to get committed. Also, this function: +/* +const char * PULSE_getMessage(UINT msg) { + static char unknown[32]; is commented out. Please leave stuff like that out of your patches, it's a little distracting. That was the only instance of that I saw in the patch, so your patch looks of generally good quality. Still, try to make your patches as clean as possible. > Should I send a message to wine-patches as well? Only if it's ready to be committed. Since you're asking for feedback, I assume that's not the case ;-) Since you're asking for feedback: what, in particular, do you want feedback on? Are you soliciting "does this work for you" feedback? Is there some aspect of the design you're not sure about? Also, I'm happy to try it, but the bug is just asking for a feature, not describing something that doesn't work without this. Is there some other criterion I can use to evaluate it? Doesn't steal audio, doesn't stutter as much, ??? --Juan

Oct. 2, 2008

Re: gdiplus: Stub GdipRecordMetafileFileName andGdipRecordMetafileFileNameI

by Nikolay Sivov

Alistair Leslie-Hughes wrote: > "Alistair Leslie-Hughes" <leslie_alistair(a)hotmail.com> wrote in message > news:48DCB2A0.9070405(a)hotmail.com... > >> Hi, >> >> Changelog: >> gdiplus: Stub GdipRecordMetafileFileName and GdipRecordMetafileFileNameI >> >> > Was there anything wrong with this patch? > > Best Regards > Alistair Leslie-Hughes > 1.GDIPCONST WCHAR* should be declared as wstr in spec 2. debug output for strings should differ

Oct. 2, 2008

Re: gdiplus: Stub GdipRecordMetafileFileName andGdipRecordMetafileFileNameI

by Alistair Leslie-Hughes

"Alistair Leslie-Hughes" <leslie_alistair(a)hotmail.com> wrote in message news:48DCB2A0.9070405(a)hotmail.com... > Hi, > > Changelog: > gdiplus: Stub GdipRecordMetafileFileName and GdipRecordMetafileFileNameI > Was there anything wrong with this patch? Best Regards Alistair Leslie-Hughes

Oct. 2, 2008

Re: Wine PulseAudio Driver

by Arthur Taylor

I have put a newer patch in bugzilla: http://bugs.winehq.org/attachment.cgi?id=16412&action=edit Currently I am looking for any feedback at all. Should I send a message to wine-patches as well? > 2. We ought to be able to use PulseAudio's ALSA emulation. If that > doesn't work, then either our ALSA driver should be fixed, or > PulseAudio's ALSA interface should be fixed. IMHO this is a situation were it makes more sense to use pulse's native API over going through ALSA's, as we aren't just trying to join ALSA-pulse, we are trying to join WaveOut-ALSA-pulse (or even dsound on top of waveout.) Currently I can match the alsa driver's perceptive quality (buzz word) on my system using this code, dsound and all. By far beats the current ALSA-pulse or ESD solutions. Thanks - Art

Oct. 2, 2008