comctl32: realloc or free and alloc
Hallo,
i currently digg in comctl32 to find why my app fails. I found that
string conversation AtoW in some places silently fails. The problem is
that the destination for string is just a fresh pointer (not NULL).
Str_SetPtrAtoW check if it is NULL pointer and if not it trys to
ReAlloc. There was no Alloc before so ReAlloc returns NULL. Because
there is no other checks, Str_SetPtrAtoW just go back without converting
the string.
Here is affected code:
dlls/comctl32/comctl32undoc.c:999
> BOOL Str_SetPtrAtoW (LPWSTR *lppDest, LPCSTR lpSrc)
> {
> TRACE("(%p %s)\n", lppDest, lpSrc);
>
> if (lpSrc) {
> INT len = MultiByteToWideChar(CP_ACP,0,lpSrc,-1,NULL,0);
> LPWSTR ptr = ReAlloc (*lppDest, len*sizeof(WCHAR));
>
> if (!ptr) {
> ERR("last error %x\n", GetLastError());
> return FALSE;
> }
> MultiByteToWideChar(CP_ACP,0,lpSrc,-1,ptr,len);
> *lppDest = ptr;
> }
> else {
> Free (*lppDest);
> *lppDest = NULL;
> }
>
> return TRUE;
> }
The question is, is it appropriate reaction of ReAlloc and if it so, may be it
make more sense to do this:
--- a/dlls/comctl32/comctl32undoc.c
+++ b/dlls/comctl32/comctl32undoc.c
@@ -998,18 +998,26 @@ INT Str_GetPtrAtoW (LPCSTR lpSrc, LPWSTR lpDest, INT
nMaxLen)
*/
BOOL Str_SetPtrAtoW (LPWSTR *lppDest, LPCSTR lpSrc)
{
-TRACE("(%p %s)\n", lppDest, lpSrc);
+TRACE("(%p, %s)\n", *lppDest, debugstr_a(lpSrc));
if (lpSrc) {
+if (*lppDest) {
+Free (*lppDest);
+*lppDest = NULL;
+}
INT len = MultiByteToWideChar(CP_ACP,0,lpSrc,-1,NULL,0);
- LPWSTR ptr = ReAlloc (*lppDest, len*sizeof(WCHAR));
+ LPWSTR ptr = Alloc (len*sizeof(WCHAR));
- if (!ptr)
+ if (!ptr) {
+ERR("Can't allocait memory size: %u,"
+" for string %s.\n", len*sizeof(WCHAR), lpSrc);
return FALSE;
+}
MultiByteToWideChar(CP_ACP,0,lpSrc,-1,ptr,len);
*lppDest = ptr;
}
else {
+TRACE("lpSrc = NULL, nothing to convert.\n");
Free (*lppDest);
*lppDest = NULL;
}
--
Regards,
Alexey
Re: wined3d: If Wine fails to acquire the focus window, allow the wndproc function to continue receiving messages.
On 30 April 2011 01:23, John Edmonds wrote:
> @@ -6559,6 +6559,7 @@ static HRESULT WINAPI
> IWineD3DDeviceImpl_Reset(IWineD3DDevice *iface,
> {
> ERR("Failed to acquire focus window, hr %#x.\n", hr);
> wined3d_swapchain_decref(swapchain);
> + This->filter_messages = filter;
> return hr;
> }
This patch looks right in principle. Note that if you're running into
this code you probably have problems elsewhere though.
Re: [PATCH] dinput: Mark internal symbols as hidden
On 30 April 2011 00:41, Stefan Dösinger wrote: > I don't know how much the effect of marking a few functions hidden changes > that > though. In the d3d code most things are already either static or hidden, and > still not using -fPIC on wined3d alone improves things. > Things like TRACEs still cause pc loads. There's the actual wine_dbg_log() call that has to go through the plt, but also the string references and the debug channel info that go through the got. I suppose that if you're crazy enough about performance to care about that kind of thing you'd already compiled those out anyway though.
Re: gecko and native msvcrt
--- On Fri, 29/4/11, Eric Pouech wrote: > Le 29/04/2011 23:07, Dan Kegel a > écrit : > > While testing a game with current gecko, I saw the > error > > > > wine: Call from 0x7bc4ad90 to unimplemented function > msvcrt.dll._snwprintf_s, > > aborting > > > > The game needed a native msvcrt, so I had installed > one with winetricks, > > and it didn't export that function. > > > > So it seems that new gecko is incompatible with old > msvcrt, > > and using native msvcrt as a workaround for bugs in > wine's > > version is going to be harder than it used to be. > > > > Time to fix more of them msvcrt bugs, I guess... > > And/or find a more up to date msvcrt.dll for > winetricks. > > > > > > > or link gecko against msvcr80 or msvcr90 instead of msvcrt > A+ This may be asking for trouble - I assume gecko is (preferably) built with mingw gcc, rather than msvc. Recently I tried to go this route - wanted to have 64-bit file access from msvc80. Turned out that binaries generated this way gets very confused by the side-by-side assembly system with msvc80 and you need to add/insert a manifest to the PE if you want to go this route, and doing manifest insertion with mingw gcc or open-source tools (rather than microsoft's manifest tool) aren't easy. Ended up just switching to use mingw-specific unix-like 64-bit file access functions instead, to stay with msvcrt (v7). (i.e. the code wouldn't then build with MSVC because it uses mingw-specific things); but I would rather have it built easy with mingw gcc - it is a small sacrifice and easy to make to drop compatibility with msvc compiler..
Re: wineoss.drv: Add mmdevapi driver.
I believe commit be332326ba8fc3def406c5f29adf04fbe9a83976 Author: Andrew Eikum Date: Wed Apr 27 09:12:36 2011 -0500 is causing the following build failures on my nightly FreeBSD testers: mmdevdrv.c:463:20: error: 'AFMT_S24_PACKED' undeclared (first use in this function) mmdevdrv.c:476:16: error: 'AFMT_FLOAT' undeclared (first use in this function) mmdevdrv.c:854:24: error: 'AFMT_FLOAT' undeclared (first use in this function) mmdevdrv.c:863:24: error: 'AFMT_S24_PACKED' undeclared (first use in this function) mmdevdrv.c:1084:24: error: 'SNDCTL_DSP_HALT' undeclared (first use in this function) Looks like some autoconfigury is not working or incomplete? Gerald
Re: [PATCH] dinput: Mark internal symbols as hidden
On Friday 29 April 2011 21:01:36 Marcus Meissner wrote: > I do not think there will be noticable speed-up. Actually, not using -fPIC gives a ~2-3% speed boost in a number of d3d benchmarks. I don't know how much the effect of marking a few functions hidden changes that though. In the d3d code most things are already either static or hidden, and still not using -fPIC on wined3d alone improves things. signature.asc Description: This is a digitally signed message part.
Re: gecko and native msvcrt
Le 29/04/2011 23:07, Dan Kegel a écrit : While testing a game with current gecko, I saw the error wine: Call from 0x7bc4ad90 to unimplemented function msvcrt.dll._snwprintf_s, aborting The game needed a native msvcrt, so I had installed one with winetricks, and it didn't export that function. So it seems that new gecko is incompatible with old msvcrt, and using native msvcrt as a workaround for bugs in wine's version is going to be harder than it used to be. Time to fix more of them msvcrt bugs, I guess... And/or find a more up to date msvcrt.dll for winetricks. or link gecko against msvcr80 or msvcr90 instead of msvcrt A+ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)
gecko and native msvcrt
While testing a game with current gecko, I saw the error wine: Call from 0x7bc4ad90 to unimplemented function msvcrt.dll._snwprintf_s, aborting The game needed a native msvcrt, so I had installed one with winetricks, and it didn't export that function. So it seems that new gecko is incompatible with old msvcrt, and using native msvcrt as a workaround for bugs in wine's version is going to be harder than it used to be. Time to fix more of them msvcrt bugs, I guess... And/or find a more up to date msvcrt.dll for winetricks.
re: msvbvm60: add stub dll
Austin wrote: > Fixes http://bugs.winehq.org/show_bug.cgi?id=19816 That bug is "Add implementation of MSVBVM60". I don't think a stub fixes it. What apps benefit from having a stub? - Dan
Re: [PATCH] dinput: Mark internal symbols as hidden
On Fri, Apr 29, 2011 at 08:12:36PM +0200, André Hentschel wrote: > Am 29.04.2011 16:18, schrieb Marcus Meissner: > > On Fri, Apr 29, 2011 at 07:38:30AM -0600, Vitaliy Margolen wrote: > >> On 04/29/2011 01:31 AM, Marcus Meissner wrote: > > > > will generate without the visibility line a PLT relocation resolving call: > > > > 045c : > > 45c: 55 push %ebp > > 45d: 89 e5 mov%esp,%ebp > > 45f: 53 push %ebx > > 460: 83 ec 04sub$0x4,%esp > > 463: e8 ef ff ff ff call 457 <__i686.get_pc_thunk.bx> > > 468: 81 c3 8c 1b 00 00 add$0x1b8c,%ebx > > 46e: e8 15 ff ff ff call 388 > > 473: 83 c4 04add$0x4,%esp > > 476: 5b pop%ebx > > 477: 5d pop%ebp > > 478: c3 ret > > > > > > but with the visibility hidden just a relative call, no relocations: > > > > 042c : > > 42c: 55 push %ebp > > 42d: 89 e5 mov%esp,%ebp > > 42f: 83 ec 08sub$0x8,%esp > > 432: e8 05 00 00 00 call 43c > > 437: c9 leave > > 438: c3 ret > > > > 043c : > > > > Ciao, Marcus > > > > Hi Marcus, > That's a very cool search&replace job, Wine reduces size and get's faster. > The question is how much faster? Hope we'll see some benchmarks from you :) I do not think there will be noticable speed-up. It however has the possibility to make funny Windows code who has troubles with %ebx changes work better. Also the size of the .sos gets smaller a bit (but just in the kilobyte range). Ciao, Marcus
Re: [PATCH] dinput: Mark internal symbols as hidden
Am 29.04.2011 16:18, schrieb Marcus Meissner: > On Fri, Apr 29, 2011 at 07:38:30AM -0600, Vitaliy Margolen wrote: >> On 04/29/2011 01:31 AM, Marcus Meissner wrote: > > will generate without the visibility line a PLT relocation resolving call: > > 045c : > 45c: 55 push %ebp > 45d: 89 e5 mov%esp,%ebp > 45f: 53 push %ebx > 460: 83 ec 04sub$0x4,%esp > 463: e8 ef ff ff ff call 457 <__i686.get_pc_thunk.bx> > 468: 81 c3 8c 1b 00 00 add$0x1b8c,%ebx > 46e: e8 15 ff ff ff call 388 > 473: 83 c4 04add$0x4,%esp > 476: 5b pop%ebx > 477: 5d pop%ebp > 478: c3 ret > > > but with the visibility hidden just a relative call, no relocations: > > 042c : > 42c: 55 push %ebp > 42d: 89 e5 mov%esp,%ebp > 42f: 83 ec 08sub$0x8,%esp > 432: e8 05 00 00 00 call 43c > 437: c9 leave > 438: c3 ret > > 043c : > > Ciao, Marcus > Hi Marcus, That's a very cool search&replace job, Wine reduces size and get's faster. The question is how much faster? Hope we'll see some benchmarks from you :) -- Best Regards, André Hentschel
Fix WineHQ bug #19762
Kai Wasserbäch wrote: >as bug #19762 is open for some time now, fully bisected, the solution applied >for >several versions in my Debian packages ([0]) without reports about breakage, I >hearby propose the reversion of commit 67631163. http://bugs.winehq.org/show_bug.cgi?id=19762#c21 That commit is another example of a commit that introduced a regression while fixing another regression in another app. This does not prove that commit wrong, however, the burden is now on you (us) to prove it in order to change git. It's not an impossible task. Even code that seemed to be backed by tests in Wine was changed when the tests were proven wrong/incomplete. For a start, it may be worth testing that other app and see if it still needs that commit in current Wine. Does anybody know the bug number of that other app's original issue? Finally, are there any tests that support either behavior? Regards, Jörg Höhle
Re: [PATCH] dinput: Mark internal symbols as hidden
On Friday, April 29, 2011 7:18:47 AM Marcus Meissner wrote: > If they are not declared hidden, the -fPIC compile generates PLT > relocations for those symbols, even if they are internal to the dll (the > compiler does not know what to export during our build). > > With the hidden attribute these get turned into just relative calls or > simpler relocations. Wouldn't it be better to compile with -fvisibility=hidden, then? Default to hidden, then explicitly mark the functions that can be exported from the .so (if Wine even needs PE exports to have "default" visibilty; I don't think it does). Functions that are *never* called from outside the module may be better marked as "internal": "Internal visibility is like hidden visibility, but with additional processor specific semantics. Unless otherwise specified by the psABI, GCC defines internal visibility to mean that a function is never called from another module. Compare this with hidden functions which, while they cannot be referenced directly by other modules, can be referenced indirectly via function pointers. By indicating that a function cannot be called from outside the module, GCC may for instance omit the load of a PIC register since it is known that the calling function loaded the correct value." http://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Function-Attributes.html#Function- Attributes FWIW, you can even mark COM functions as "hidden". As noteded above, hidden functions can still be called from the outside (through function pointers, like vtables), they just aren't able to be directly referenced.
Re: Fix WineHQ bug #19762
On 29 April 2011 12:29, Kai Wasserbäch wrote: > several versions in my Debian packages ([0]) without reports about breakage, I > hearby propose the reversion of commit 67631163. Wylda tried to reach the I don't think reverting that commit is the right thing to do, it's just going to break something else instead.
Re: [PATCH] dinput: Mark internal symbols as hidden
On Fri, Apr 29, 2011 at 07:38:30AM -0600, Vitaliy Margolen wrote:
> On 04/29/2011 01:31 AM, Marcus Meissner wrote:
> >---
> > dlls/dinput/device_private.h | 148
> > +-
> > 1 files changed, 74 insertions(+), 74 deletions(-)
>
> What exactly is this supposed to to? This is inside header file,
> everything declared in it is internal to dinput. Why do we need some
> extra declarations on it?
If they are not declared hidden, the -fPIC compile generates PLT relocations
for those symbols, even if they are internal to the dll (the compiler does not
know what to export during our build).
With the hidden attribute these get turned into just relative calls or
simpler relocations.
On x86 this will also save use of the %ebx register.
So e.g. this piece of code:
extern int g(void) __attribute__((__visibility__("hidden")));
int f() {
return g();
}
will generate without the visibility line a PLT relocation resolving call:
045c :
45c: 55 push %ebp
45d: 89 e5 mov%esp,%ebp
45f: 53 push %ebx
460: 83 ec 04sub$0x4,%esp
463: e8 ef ff ff ff call 457 <__i686.get_pc_thunk.bx>
468: 81 c3 8c 1b 00 00 add$0x1b8c,%ebx
46e: e8 15 ff ff ff call 388
473: 83 c4 04add$0x4,%esp
476: 5b pop%ebx
477: 5d pop%ebp
478: c3 ret
but with the visibility hidden just a relative call, no relocations:
042c :
42c: 55 push %ebp
42d: 89 e5 mov%esp,%ebp
42f: 83 ec 08sub$0x8,%esp
432: e8 05 00 00 00 call 43c
437: c9 leave
438: c3 ret
043c :
Ciao, Marcus
Re: [PATCH] dinput: Mark internal symbols as hidden
On 04/29/2011 01:31 AM, Marcus Meissner wrote: --- dlls/dinput/device_private.h | 148 +- 1 files changed, 74 insertions(+), 74 deletions(-) What exactly is this supposed to to? This is inside header file, everything declared in it is internal to dinput. Why do we need some extra declarations on it? Vitaliy.
Re: include: Generate rmxftmpl.h from rmxftmpl.x using new build tool. (try 3)
Hello, why did you use C++ comments ? A+ David +xof 0302txt 0064 > +// Copyright (C) 2011 Dylan Smith > +// > +// This library is free software; you can redistribute it and/or > > > >
Re: [1/5] wined3d: Add missing temporary variable declaration to ARB shader backend.
Am 29.04.2011 um 00:39 schrieb Matteo Bruni: > 2011/4/29 Stefan Dösinger : >>> +shader_addline(buffer, "TEMP TB;\n"); >> Which shader instruction uses this? Afair vertex shaders should only use TA, >> but there's no proper infrastructure that manages that. >> > > AFAICS, TB could get used in shader_hw_pow(): > > ... >shader_arb_get_src_param(ins, &ins->src[1], 1, src1); > ... > > where the '1' in the third argument means that > shader_arb_get_src_param() is allowed to use TB as temporary storage. > There may be other pieces of code with the same assumption, I didn't > look deeper. Ok PGP.sig Description: Signierter Teil der Nachricht
Re: msvcrt: fix utime to correctly handle a NULL-pointer for the utimbuf structure field
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=10627 Your paranoid android. === WINEBUILD (build) === Patch failed to apply
Re: po: Update French translation
On Fri, Apr 29, 2011 at 08:04, Nicolas Le Cam wrote: > Hi Frédéric, > > Le 29 avril 2011 00:16, Frédéric Delanoy a écrit > : >> #: winerror.mc:441 >> msgid "No more search handles\n" >> -msgstr "Il n'y a plus de descripteurs de recherche\n" >> +msgstr "Il n'y a plus de descripteur de recherche\n" > This one isn't correct. There's not only one handle when available but > many so it should take an 's' in the negative form too. Right. Fixed that and resent patch. > Thanks for your work ! > -- > Nicolas Le Cam Frédéric Delanoy
