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

Re: dlls/winex11.drv/dib.c: initialize two warnings (RESEND)

by Alexandre Julliard

Gerald Pfeifer <gerald(a)pfeifer.com> writes: > I looked into this in more detail, and now remember why I couldn't > do this. > > From X11DRV_GetDIBits() we invoke DIB_GetBitmapInfo() with these > two variables being passed by addresses which in turns forwards to > DIB_GetBitmapInfoEx(). And there we have a default case of > > ERR("(%d): unknown/wrong size for header\n", header->biSize ); > return -1; > > where indeed height and width are not initialized, so GCC 4.5 is right. No, because the error case causes the caller function to return without using them. The idea was to reorder the code so that gcc can figure this out, as it does already in the other cases. -- Alexandre Julliard julliard(a)winehq.org

Oct. 31, 2009

Re: Generate a WM_MOUSEMOVE event when warping the mouse

by Vitaliy Margolen

JesusFreke wrote: > I'll be honest, I don't know if this is the correct way to fix this issue. > But with this patch, the mouse movement is working perfectly. Have you tried disabling warping in dinput? Your patch is an outright hack. It does something that is plain wrong. I do know that area of the code really really good. As-is calling SetCursorPos already generates WM_MOUSEMOVE (when cursor position change comes back from X) which it shouldn't. You are adding an additional one - which is double wrong. I guess this is the case of two identical errors masking each-other resulting in non-error. Vitaliy.

Oct. 31, 2009

Re: Generate a WM_MOUSEMOVE event when warping the mouse

by Roderick Colenbrander

On Sat, Oct 31, 2009 at 5:37 PM, JesusFreke <jesusfreke(a)jesusfreke.com> wrote: > > > On Sat, Oct 31, 2009 at 11:00 AM, Vitaliy Margolen <wine-devel(a)kievinfo.com> > wrote: >> >> JesusFreke wrote: >> > --- >> > dlls/winex11.drv/mouse.c | 1 + >> > 1 files changed, 1 insertions(+), 0 deletions(-) >> > >> > diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c >> > index 60351e5..7c24c64 100644 >> > --- a/dlls/winex11.drv/mouse.c >> > +++ b/dlls/winex11.drv/mouse.c >> > @@ -1004,6 +1004,7 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) >> > XFlush( display ); /* avoids bad mouse lag in games that do their >> > own mouse warping */ >> > cursor_pos = pt; >> > wine_tsx11_unlock(); >> > + queue_raw_mouse_message( WM_MOUSEMOVE, NULL, x, y, 0, >> > GetCurrentTime(), 0, 0 ); >> > return TRUE; >> > } >> > >> Can you explain why do you need this patch? What exactly is it fixing? And >> have you run any tests to verify this? I'm guessing no, because you >> ignored >> that Wine will generate mouse-move event when this operation comes back to >> Wine from X server. > > Sorry, first time posting a patch. I thought I had included a introduction > to the patch with git send-email. > > In any case, this is to fix a mouse issue in AION. The issue occurs when you > right-click and drag, which moves the camera. The game frequently warps the > mouse back to the center of the screen, and the camera movement is very > jittery, like it might if you had manually moved the mouse back to the > center of the screen (instantly). Another symptom is that if you begin the > right-click drag near the top or the bottom of the screen, the camera will > instantly pan down or up, like you had instantly dragged from the start > location to the center of the y-axis. > > I'll be honest, I don't know if this is the correct way to fix this issue. > But with this patch, the mouse movement is working perfectly. > > If you have any suggestions for a better fix, I would be happy to code it up > and test it out with Aion. > > (sorry for the double-email Vitaliy - I just realized that I didn't > reply-all. And yeah, I know top posting is generally frowned upon. Sorry! :) > ) > I don't know this code that well but to me it looked like a hack as well (as I guess it will also send that event back to the app and the app shouldn't hear anything about our hacks.). These the days issue can be fixed properly on the latest Xservers which support xinput2. We need to add support for it to Wine and that would fix the issue properly. The problem is that the current code needs to be redesigned quite a bit (some part needs to be moved to explorer I believe) and then the new code can be added. Paul Hampton did a lot of work on this a while ago but I think he didn't have time to finish it. He must have code for it and if you want you could continue it (perhaps with his assistance) but it takes some effort I think. Roderick

Oct. 31, 2009

Re: mciwave: need to serialize record and play

by Eric Pouech

Joerg-Cyril.Hoehle(a)t-systems.com a écrit : > Eric, > > I wrote: > >> However, it[Wine's MCI] still hangs in the full tests. I believe the last bug in mciwave >> will only be nailed down once the play and record loops manage to talk robustly >> with other commands such as pause and close. >> > > A recent idea of mine is to build upon your thread split work and > - ensure at most one player thread is running at any time; > - by adding a wmw->thread slot; > - and stopping a playing thread before the next play or record starts a new one. > > This might cause a larger delay when issuing > play mysound; play mysound from 0; play mysound from 500 > which would require thread destruction and creation, which is much more > expensive than just bumping wmw->dwPosition (mor or less safely) while another > thread is playing, or starting a new one while the old one is not finished yet > (as is currently done, causing subtle bugs). > > What do you think? > > BTW, please don't change mciwave now as I still have many patches in the queue. > > I'm lagging answering my email yes, actually, only one thread should be running in background, so what you suggest sounds fine A+ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)

Oct. 31, 2009

Re: Generate a WM_MOUSEMOVE event when warping the mouse

by JesusFreke

On Sat, Oct 31, 2009 at 11:00 AM, Vitaliy Margolen <wine-devel(a)kievinfo.com>wrote: > JesusFreke wrote: > > --- > > dlls/winex11.drv/mouse.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c > > index 60351e5..7c24c64 100644 > > --- a/dlls/winex11.drv/mouse.c > > +++ b/dlls/winex11.drv/mouse.c > > @@ -1004,6 +1004,7 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) > > XFlush( display ); /* avoids bad mouse lag in games that do their > own mouse warping */ > > cursor_pos = pt; > > wine_tsx11_unlock(); > > + queue_raw_mouse_message( WM_MOUSEMOVE, NULL, x, y, 0, > GetCurrentTime(), 0, 0 ); > > return TRUE; > > } > > > Can you explain why do you need this patch? What exactly is it fixing? And > have you run any tests to verify this? I'm guessing no, because you ignored > that Wine will generate mouse-move event when this operation comes back to > Wine from X server. > Sorry, first time posting a patch. I thought I had included a introduction to the patch with git send-email. In any case, this is to fix a mouse issue in AION. The issue occurs when you right-click and drag, which moves the camera. The game frequently warps the mouse back to the center of the screen, and the camera movement is very jittery, like it might if you had manually moved the mouse back to the center of the screen (instantly). Another symptom is that if you begin the right-click drag near the top or the bottom of the screen, the camera will instantly pan down or up, like you had instantly dragged from the start location to the center of the y-axis. I'll be honest, I don't know if this is the correct way to fix this issue. But with this patch, the mouse movement is working perfectly. If you have any suggestions for a better fix, I would be happy to code it up and test it out with Aion. (sorry for the double-email Vitaliy - I just realized that I didn't reply-all. And yeah, I know top posting is generally frowned upon. Sorry! :) )

Oct. 31, 2009

Re: Playing ULAW sample correctly?

by Eric Pouech

Joerg-Cyril.Hoehle(a)t-systems.com a écrit : > Hi, > > >> IIRC, I never tested µ-law wave file, so it may well be that the ACM >> converter is broken >> > > What is the policy for including sample multimedia files in Wine? > E.g., I remember tests being skipped because msrle.avi is not present: > http://www.winehq.org/pipermail/wine-devel/2008-October/070112.html > > It is not acceptable that µ-law code bit-rots because of lack of continuous > testing (assuming it worked the day it was checked in). > > Regards, > Jörg höhle > > > actually, you can add a (small) wave file in the test program (and create on the fly the needed file) we already test some .exe this way in winetest A+ -- Eric Pouech "The problem with designing something completely foolproof is to underestimate the ingenuity of a complete idiot." (Douglas Adams)

Oct. 31, 2009

Re: Nate Gallaher : msi: Add tablename tracking to VIEW_find_column

by Dan Kegel

On Sat, Oct 31, 2009 at 8:02 AM, Nathan Gallaher <ngallaher(a)deepthought.org> wrote: > Could someone give me a pointer as to how you reproduce this trace? I've > spent some time fiddling with valgrind, the WINETEST_WRAPPER env var, and > --enable-maintainer-mode, all to no avail. I use http://code.google.com/p/winezeug/source/browse/trunk/valgrind/valgrind-dai… but you can more simply do wine notepad & cd dlls/msi/tests valgrind --trace-children=yes --track-origins=yes wine msi_test.exe.so db.c Today that finds one valgrind error: ==21129== Conditional jump or move depends on uninitialised value(s) ==21129== at 0xCD6B96A: RtlFreeHeap (heap.c:1460) ==21129== by 0xD700437: msi_free (msipriv.h:1099) ==21129== by 0xD70051F: VIEW_find_column (msiquery.c:82) ==21129== by 0xD714479: SELECT_AddColumn (select.c:392) ==21129== by 0xD71466F: SELECT_CreateView (select.c:436) ==21129== by 0xD6F07CD: INSERT_CreateView (insert.c:393) ==21129== by 0xD72D4B6: sql_parse (sql.y:149) ==21129== by 0xD72ED6A: MSI_ParseSQL (sql.y:960) ==21129== by 0xD70083C: MSI_DatabaseOpenViewW (msiquery.c:137) ==21129== by 0xD700C90: MsiDatabaseOpenViewW (msiquery.c:279) ==21129== by 0xD70060F: MsiDatabaseOpenViewA (msiquery.c:110) ==21129== by 0xD541575: test_streamtable (db.c:1443) ==21129== Uninitialised value was created by a stack allocation ==21129== at 0xD700443: VIEW_find_column (msiquery.c:60) If you also want to see memory leaks, add --leak-check=full - Dan

Oct. 31, 2009

Re: Generate a WM_MOUSEMOVE event when warping the mouse

by Vitaliy Margolen

JesusFreke wrote: > --- > dlls/winex11.drv/mouse.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/dlls/winex11.drv/mouse.c b/dlls/winex11.drv/mouse.c > index 60351e5..7c24c64 100644 > --- a/dlls/winex11.drv/mouse.c > +++ b/dlls/winex11.drv/mouse.c > @@ -1004,6 +1004,7 @@ BOOL CDECL X11DRV_SetCursorPos( INT x, INT y ) > XFlush( display ); /* avoids bad mouse lag in games that do their own mouse warping */ > cursor_pos = pt; > wine_tsx11_unlock(); > + queue_raw_mouse_message( WM_MOUSEMOVE, NULL, x, y, 0, GetCurrentTime(), 0, 0 ); > return TRUE; > } > Can you explain why do you need this patch? What exactly is it fixing? And have you run any tests to verify this? I'm guessing no, because you ignored that Wine will generate mouse-move event when this operation comes back to Wine from X server.

Oct. 31, 2009

Re: msvcrt: do not overlap in strcpy (valgrind) (try 2)

by Vitaliy Margolen

André Hentschel wrote: > Vitaliy Margolen schrieb: >> André Hentschel wrote: >>> diff --git a/dlls/msvcrt/math.c b/dlls/msvcrt/math.c >>> index 010e1a6..08eaf73 100644 >>> --- a/dlls/msvcrt/math.c >>> +++ b/dlls/msvcrt/math.c >>> @@ -856,7 +856,7 @@ char * CDECL _ecvt( double number, int ndigits, int *decpt, int *sign ) >>> snprintf(data->efcvt_buffer, 80, "%.*le", prec - 1, number); >>> /* take the decimal "point away */ >>> if( prec != 1) >>> - strcpy( data->efcvt_buffer + 1, data->efcvt_buffer + 2); >>> + memmove( data->efcvt_buffer + 1, data->efcvt_buffer + 2, strlen(data->efcvt_buffer) ); >> You don't need additional strlen. You already have a length from snprintf >> (that's returned but not assigned to anything). >> >> > thx again, really didnt got that. > > - strcpy( data->efcvt_buffer + 1, data->efcvt_buffer + 2); > + memmove( data->efcvt_buffer + 1, data->efcvt_buffer + 2, len ); You still have a problem here. You moving 1 byte too many. Remember, memmove is not a string function and doesn't care about '\0' characters. Vitaliy.

Oct. 31, 2009

re: Nate Gallaher : msi: Add tablename tracking to VIEW_find_column

by Nathan Gallaher

Could someone give me a pointer as to how you reproduce this trace? I've spent some time fiddling with valgrind, the WINETEST_WRAPPER env var, and --enable-maintainer-mode, all to no avail. ~Nate On Wed, 28 Oct 2009, Dan Kegel wrote: > Hi Nate, > your patch seems to have caused new valgrind warnings, could you have a look? > > Found via > http://kegel.com/wine/valgrind/logs/2009-10-27-08.06/diff-msi_db.txt > Details at > http://kegel.com/wine/valgrind/logs/2009-10-27-08.06/vg-msi_db.txt > > Conditional jump or move depends on uninitialised value(s) > at RtlFreeHeap (heap.c:1460) > by msi_free (msipriv.h:1099) > by VIEW_find_column (msiquery.c:82) > by SELECT_AddColumn (select.c:392) > by SELECT_CreateView (select.c:436) > by INSERT_CreateView (insert.c:393) > by sql_parse (sql.y:149) > by MSI_ParseSQL (sql.y:960) > by MSI_DatabaseOpenViewW (msiquery.c:137) > by MsiDatabaseOpenViewW (msiquery.c:279) > by MsiDatabaseOpenViewA (msiquery.c:110) > by test_streamtable (db.c:1443) > by func_db (db.c:8546) > by run_test (test.h:535) > by main (test.h:585) > Uninitialised value was created by a stack allocation > at VIEW_find_column (msiquery.c:60) > ... > 2 bytes in 1 blocks are definitely lost > at notify_alloc (heap.c:214) > by RtlAllocateHeap (heap.c:1421) > by msi_alloc_zero (msipriv.h:1082) > by table_create_new_row (table.c:1504) > by TABLE_insert_row (table.c:1712) > by TABLE_modify (table.c:1860) > by MSI_ViewModify (msiquery.c:623) > by MsiViewModify (msiquery.c:644) > by test_viewmodify_delete_temporary (db.c:5892) > by func_db (db.c:8570) > by run_test (test.h:535) > by main (test.h:585) > ... > 4 bytes in 1 blocks are definitely lost > at notify_alloc (heap.c:214) > by RtlAllocateHeap (heap.c:1421) > by msi_alloc (msipriv.h:1076) > by strdupW (msipriv.h:1132) > by msi_create_table (table.c:664) > by CREATE_execute (create.c:72) > by MSI_ViewExecute (msiquery.c:462) > by MsiViewExecute (msiquery.c:488) > by run_query (db.c:177) > by test_droptable (db.c:7073) > by func_db (db.c:8580) > by run_test (test.h:535) > by main (test.h:585) > ... > 4 bytes in 1 blocks are definitely lost > at notify_alloc (heap.c:214) > by RtlAllocateHeap (heap.c:1421) > by msi_alloc (msipriv.h:1076) > by strdupW (msipriv.h:1132) > by msi_makestring (table.c:943) > by get_tablecolumns (table.c:1016) > by table_get_column_info (table.c:608) > by msi_update_table_columns (table.c:1053) > by TABLE_remove_column (table.c:2040) > by TABLE_drop (table.c:2293) > by DROP_execute (drop.c:60) > by MSI_ViewExecute (msiquery.c:462) > by MsiViewExecute (msiquery.c:488) > by run_query (db.c:177) > by test_dbmerge (db.c:7408) > by func_db (db.c:8581) > by run_test (test.h:535) > by main (test.h:585) > ... > 224 bytes in 28 blocks are definitely lost > at notify_alloc (heap.c:214) > by RtlAllocateHeap (heap.c:1421) > by msi_alloc (msipriv.h:1076) > by strdupW (msipriv.h:1132) > by msi_makestring (table.c:943) > by get_tablecolumns (table.c:1014) > by table_get_column_info (table.c:608) > by msi_update_table_columns (table.c:1053) > by TABLE_remove_column (table.c:2040) > by TABLE_drop (table.c:2293) > by DROP_execute (drop.c:60) > by MSI_ViewExecute (msiquery.c:462) > by MsiViewExecute (msiquery.c:488) > by run_query (db.c:177) > by test_dbmerge (db.c:7385) > by func_db (db.c:8581) > by run_test (test.h:535) > by main (test.h:585)k > > It also fails here: > > db.c:5260: Test failed: Expected ERROR_NO_MORE_ITEMS, got 0 > db.c:5336: Test failed: Expected 4, got 8 > db.c:5337: Test failed: Unexpected data > db.c:5352: Test failed: Unexpected data > db.c:5378: Test failed: Expected 64, got 68 > db.c:5379: Test failed: Unexpected data > db.c:6993: Test failed: Expected "", got "grape" > db.c:6994: Test failed: Expected 0, got 5 > db.c:7052: Test failed: Expected "", got "grape" > db.c:7053: Test failed: Expected 0, got 5 >

Oct. 31, 2009