Re: RFC: Remove auto-scan of ALSA devices from winealsa.drv
On 12/11/2012 08:46 AM, Henri Verbeet wrote: On 11 December 2012 16:05, joerg-cyril.hoe...@t-systems.com wrote: Cost to users: Users with a working ALSA device default should experience no drawback, only benefits. I believe this is the vast majority of users. Users that edit their ~/.asoundrc to define other devices without simultaneously overriding !default will have to additionally edit the Wine registry to name their working ALSA capture and render devices, e.g. asnoop and amix. It will also pretty much just remove device selection on setup with multiple audio devices, which is actually fairly common these days with USB audio devices and HDMI outputs on graphics cards. I think the correct approach would to work with upstream ALSA to fix things, instead of just removing device enumeration. Can winecfg use a special interface into winealsa co to enumerate available devices? But change winealsa to use default device otherwise? - Vitaliy
Re: Disabling joysticks with registry keys and joy.cpl
On 06/18/2012 12:07 PM, Lucas Zawacki wrote: The bottom line is: can I add platform dependent code in joy.cpl for this purpose? If that's not an option, do you have some other way to do through joy.cpl or should it be registry only? If you use generated device GUIDs that won't be as platform dependent but at the same time will uniquely identify which joystick you want to disable. If you worry about devices moving around and getting different IDs - then use part of GUID to identify driver and add device name to identify the joystick. The most common use case is to have only one joystick via best working driver. Instead of 2 joysticks representing the same exact device. This means all would almost never be used. Another feature could be defining the order of enumeration without disabling any joysticks. In a sense creating a default device. This is an extra bonus and can be accomplished by disabling unwanted devices. So it's more of nice to have category. Vitaliy.
Re: Implementing joystick ids in DirectInput
On 05/28/2012 12:53 PM, Lucas Zawacki wrote: The solution I came up is in the attached patch, it's a little weird It's not just weird it's incorrect. You shouldn't use that list or rely on order of devices in that list. Instead you should use get_joystick_index() to get the index. It's not exactly clear where this ID is used so I doubt returning the same ID for different drivers will change anything Vitaliy.
New forum software
Can I ask when we'll have the new forum software installed? It's been discussed ages ago and we still using the same old crappy version. Vitaliy.
Re: Request for help/advice in investigation of one interesting huge FPS regression bug
On 04/15/2012 04:44 PM, Alexey Loukianov wrote: With oprofile I hit another trouble - it seems that this tool is unable to fetch symbols from libGL Of course it won't - they are binary blobs from Nvidia. Not much to see there. All you really looking for are time spent in that library. Vitaliy.
Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168
On 04/09/2012 12:14 PM, Kornél Pál wrote: I think this would work. (But don't know whether it was accepted.) No, it won't work. Wine itself (ntdll kernel32) accesses it. Vitaliy.
Re: [1/2] comctl32/tests: Added tests for mouse events handling (try 3)
On 04/08/2012 01:44 PM, Daniel Jelinski wrote: Hardware messages are an alternative, but I couldn't find any relevant examples, so I followed the path of least resistance. These tests you wrote won't work. Emulating mouse up/down events with anything other then hardware messages will not exercise the entire chain. For example search for mouse_event or SendInput. Vitaliy.
Re: My GSoC proposition: Raw Input
On 04/04/2012 12:34 PM, Henri Verbeet wrote: On 4 April 2012 20:21, Eryk Wieliczkoewde...@gmail.com wrote: I've been thinking about joining GSoC as a student this year. As an idea I propose Raw Input, and I seek your advise. Is this topic good, what are possible catches, etc. I think that it might be possible to finally do a decent implementation, that would (besides mouse and keyboard) also support joysticks and possibly some other input devices - tablets and multiple mice and keyboards. What do you think about this topic? My impression of Raw Input is that it's the kind of thing that's fairly hard even for someone already familiar with all the areas of Wine involved. That probably makes it unsuitable for GSoC, although I suppose there are no hard rules for what is or isn't suitable. It's not really that hard. The hard part is getting design accepted by AJ. Raw Input has to heavily interact with wine server. And any changes there are hard to get right and eventually accepted. So, if you have an idea of how you want to do it. And this idea is good enough to go in - that's fine. But be ready to completely redesign re-code some pieces if design won't be accepted. Vitaliy.
Re: winex11.drv: Do not trash existing clip rectangle if XInput2 is not available.
On 03/27/2012 02:29 AM, Alexandre Julliard wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: If you have suggestions on how to do this differently please explain them. Dinput needs to talk to x11drv directly to get xi2 events. Same for requests to clip cursor in exclusive mode. The current method of using hooks can stay for backwards compatibility. This can also apply to keyboard events for dinput as well. They main issue here is that you need a thread not directly attached to any window pulling all X's events. Dinput already have one such thread. You just need to let it talk to X. Vitaliy.
Re: winex11.drv: Do not trash existing clip rectangle if XInput2 is not available.
On 03/27/2012 08:04 AM, Alexandre Julliard wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: Dinput needs to talk to x11drv directly to get xi2 events. Same for requests to clip cursor in exclusive mode. The current method of using hooks can stay for backwards compatibility. I'm not at all convinced that this would be an improvement, particularly since you still need the events to follow the normal path for apps that don't use dinput. In exclusive mode dinput consumes those events - they should not be making it to the actual application. So it's not a problem there. In background mode they are duplicated - again not an issue. It's a separate data feed into dinput of key press/releases and relative position changes - gets rid of all the complexity related to absolute - relative calculations. And it's a possible source for RawInput whenever it will be implemented. It can receive background kbd mouse events as well. Vitaliy.
Re: winex11.drv: Do not trash existing clip rectangle if XInput2 is not available.
On 03/26/2012 06:14 AM, Alexandre Julliard wrote: Dmitry Timoshkovdmi...@baikal.ru writes: This patch makes dlls/user32/input.c and dlls/user32/monitor.c tests pass on a system without XInput2. It will break dinput, we rely on the clip rectangle being reset. I'd say it again, dinput should not warp mouse under any circumstances. Also, there is such a thing as non-exclusive background mode. Vitaliy.
Re: winex11.drv: Do not trash existing clip rectangle if XInput2 is not available.
On 03/26/2012 08:15 AM, Alexandre Julliard wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: On 03/26/2012 06:14 AM, Alexandre Julliard wrote: Dmitry Timoshkovdmi...@baikal.ru writes: This patch makes dlls/user32/input.c and dlls/user32/monitor.c tests pass on a system without XInput2. It will break dinput, we rely on the clip rectangle being reset. I'd say it again, dinput should not warp mouse under any circumstances. Also, there is such a thing as non-exclusive background mode. I fail to see how this is relevant here, care to explain? If Dmitry fixes a real bug that means dinput shouldn't depend on broken behavior. And I'm questioning that exact behavior which shouldn't have been there in the first place. Dinput's exclusive mode works regardless of what ClipCursor is set to. Vitaliy.
Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168
On 03/24/2012 08:56 AM, Kornél Pál wrote: On 3/22/2012 2:19 PM, Vitaliy Margolen wrote: Since there are a plenty of ways to measure elapsed time, I don't think that this specific way should generally be prohibited. I'm not saying it should be prohibited. I'm saying it fixes only one app and potentially breaks 100s more... Vitaliy
Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168
On 03/24/2012 01:09 PM, Joey Yandle wrote: Any code written for windows expects these values to update every 15.6 ms. Exactly. Wine's wineserver fake kernel in form of ntoskernl can not work with native speed by definition. For example even earliest versions of safedisk (1.5) were really picky about how long some kernel calls take. This is to protect against debugger. When numbers don't increment (as happens now) protection is happy. When they start to increment, even on fast PCs round trip user-space-wineserver-ntoskrnl will take way longer then it should. Of course this is all based on research I did 5 years ago. But surprisingly enough those old Safedisk versions still work, assuming you have supported gcc version, and few other requirements. Vitaliy
Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168
On 03/21/2012 10:19 PM, Joey Yandle wrote: It's been explained several times already that any approach to share data between wineserver and clients is going to be rejected. Can you point me to such an explanation? I joined wine-devel just before posting my RFC. If this has been discussed previously, I'd prefer to get the context before wasting my and others' time. BTW one more thing, this change will most likely break number of copy protection systems. Such as safedisk. They use user shared date times to estimate time it took for some kernel operations. And some of those time intervals are really tight. -Vitaliy
Re: RFC: KUSER_SHARED_DATA update patch to fix bug 29168
On 03/21/2012 10:19 PM, Joey Yandle wrote: If not, can you look at the APC based patch I posted and let me know how to make the callbacks happen with more precise timing? Yes, that is the only possible way to do this currently in Wine. Separate thread is a no-go. It will break some applications. And no, there is no way to make it more accurate. See other posts about this topic CreateTimerQueue. - Vitaliy.
Re: Updating GSoC proposal
On 03/20/2012 03:43 AM, Jacek Caban wrote: Hi all, GSoC is starting this year and, if we want to have good applications, we need to update our proposals. Usually the most attention is directed into adding new ones, while we keep obviously bad (or just bad IMO) proposals on the page. I'm planning to remove following project proposals: Security - implement sandboxing Theming - Implement Wine theming support NTDLL - support performance registry keys Winelib Aware Scons (or cmake) Cleanup Winemenubuilder to support generating Application Bundles on Mac OS X Wine-based application virtualization I don't think we should remove all of them. These projects should probably stay: Theming - there are few things that could be done here: 1. Implement comctl v6 controls. Probably way too much for one GSoC project 2. Interact with host desktop environment to sync at least colors. This one is more of a research then codding. Plus convincing AJ that we want that code in. Also questionable if it can be done in pure C. Of course it can be just a standalone program. Sandboxing - a great project for a security researcher. However the scope of it can be huge. Thin app - depends on how to look at it: 1. Not directly helping Wine but along the same lines as Windows' wined3d. 2. Needs some tricky functions implemented. 3. Needs a way to build and incorporate win-pe wine dlls into Wine. This is a tough one but helps some some applications as well. Vitaliy.
Re: advapi32: Correct tests under WOW64 (try 2)
On 03/12/2012 03:23 AM, Alexandre Julliard wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: On 03/10/2012 09:44 AM, André Hentschel wrote: Am 10.03.2012 17:37, schrieb Vitaliy Margolen: On 03/10/2012 12:36 AM, Alistair Leslie-Hughes wrote: Hi, Marked test as broken if it returns true. +/* WoW64 return TRUE with the LastError set */ +ok((!ret GetLastError() == ERROR_INSUFFICIENT_BUFFER) || broken(ret), %d with error %d\n, ret, GetLastError()); This is incorrect. You can check or report result of GetLastError(). But not both at the same time. You have to store the result in additional variable. Then compare and report it. Really? That's quite common practice. The value won't change between the two GetLastError() calls. It sure can change. Of course not. The only thing which is bad is if you test a function inside the ok() macro and report the last error in the same ok(). Isn't that what I said? To clarify, it applies to any printf type function that has call to GetLastError() as a parameter. There's nothing magical about printf. As always you right, my apologies. Was thinking about other code cleanup that was moving function calls outside of ok() check when also printing GetLastError(). Vitaliy.
Re: Instructions for using JACK with with Wine's ALSA output on wiki: reverted twice
On 03/12/2012 06:11 AM, Andrew Eikum wrote: Vitaliy, On Sun, Mar 11, 2012 at 10:15:40AM -0600, Vitaliy Margolen wrote: That information simply does not belong on Wine wiki. It talks about making Jack work as Alsa sound device. This has nothing to do with Wine in particular. Especially that he posted the link to an external wiki which has most of this information. Let me make it a bit more clear, Wine is huge and it uses lots and lots of system libraries, packages, subsystems, features, and so on. Unless some information directly related to Wine it should not be posted on the Wine wiki. Or we'll end up with a huge collection of generic Linux/OS X/BSD/ARM/etc articles that will rot over time. Next time you're going to delete an entire half of a Wiki page that someone clearly cares about and maintains (see the edit history), please contact that someone first so we can discuss what should be done about the information that doesn't belong there. Sorry, but I'm not going to do that. If you think that some, not Wine related, information belongs on Wine wiki, you better bring that question here. We already shot down a lot of attempts to host all sort of junk, including win API documentation. Which, if anything, more relates to Wine then how to make one sound device work like another sound device. I agree that the Wiki isn't the best place for that code, and I was planning on getting around to fixing it. In the meantime, I had linked to that code in Bug 29294 and now it's not there for users to find. There is no emergency here that could not have been better resolved by a discussion over email. What prevent you from attaching that code to the bug that needs it? And discussion with whom? That person has no wiki personal page, nor was he using a known nik associated with an e-mail. The fact that you've been specifically called out in two threads in one weekend might be a hint that it is _your_ behavior that is inappropriate. I've been called out in more then two threads, that doesn't bother me. That just an indication that no one else wants to do this work. Vitaliy.
Re: Are we the reason Apple isn't helping us?
First of all that me address your subject: I really doubt Apple will ever help Wine, regardless what happens on our bugzilla. Apple have pretty specific predisposition to open source and they sure won't change it because of one unsuccessful bug report. On 03/10/2012 09:33 PM, Charles Davis wrote: In your defense, I realize that Vitaliy's behavior is not representative of Wine developers as a whole, and that Jeremy's behavior wasn't exactly golden, either. I also realize that there really is no build breakage resulting from the bug, because we don't include any Objective-C headers. He was convinced that there could have been breakage, however. You--AJ, I'm talking to you in particular--did not adequately explain to him why this didn't break the build. Instead, you--Vitaliy in particular--practically yelled at him--for trying to help! By this point, he's likely told his colleagues at Apple about his experience. With guys like Vitaliy developing Wine, is it any wonder Apple doesn't want to help? Are we, the developers, the real reason Apple isn't contributing? For that matter, are we the real reason more people *in general* aren't contributing? Can you show me exactly where did I yell at him? Or where I wasn't polite enough in explaining that his behavior on _our_ bugzilla is not appropriate and he either sticks to it, or don't report bugs to us anymore? When everyone, including AJ telling the person what you reporting is not an error from our POV, unless you can come up with a broken application, a test program/code, or any other proof on a contrary we deem such bugs invalid. This was the rule of our bugzilla for years. I hope this does not require yet another explanation why. And one last thing, all I did is to stop an endless reopening of that bug. If you think there is/was another way to make Jeremy understand why that bug was invalid lets hear it. Vitaliy.
Re: Instructions for using JACK with with Wine's ALSA output on wiki: reverted twice
On 03/11/2012 06:31 AM, Alex Bradbury wrote: Recently user 'falktx' posted instructions on the Sound wiki page on how to set up an ALSA to JACK bridge that would work with Wine. He did this at the suggestion of aeikum: 14:45@aeikum okay. so it is possible to get this working correctly. that's good to know 14:46 falktx I can make a small package with my setup scripts 14:47@aeikum that might be handy to put up on the sound wiki page http://wiki.winehq.org/Sound If you look at the page edit history, you'll see the content has been reverted twice by Vitaliy without comment: http://wiki.winehq.org/Sound?action=info Please can you (wine developers) clarify your position on this. It seems to be useful content for the wiki that will benefit a number of users and I can see no justification for deleting it. It could be argued it clutters up the Sound wiki page, in which case it should be moved to a separate page linked to from there. That information simply does not belong on Wine wiki. It talks about making Jack work as Alsa sound device. This has nothing to do with Wine in particular. Especially that he posted the link to an external wiki which has most of this information. Let me make it a bit more clear, Wine is huge and it uses lots and lots of system libraries, packages, subsystems, features, and so on. Unless some information directly related to Wine it should not be posted on the Wine wiki. Or we'll end up with a huge collection of generic Linux/OS X/BSD/ARM/etc articles that will rot over time. Vitaliy.
Re: Are we the reason Apple isn't helping us?
On 03/11/2012 12:52 PM, Dan Kegel wrote: Vitaliy wrote: Can you show me exactly where did I yell at him? Or where I wasn't polite enough I think it was http://bugs.winehq.org/show_bug.cgi?id=29922#c14 where you wrote If that is yelling for you, then I'm definitely not understanding English... Elaborate what _exact_ part of that was yelling? There ought to be some wiggle room granted people who work at important neighboring projects who are unable for whatever reason to follow Wine's usual conventions; at the very least, That's what happens most of the time. If those other people actually willing to work with us. And don't reply to the request of showing what's the problem is with: ---snip--- AJ: Then file a proper bug, showing the exact error and the exact compiler and OS version you are using. Jeremy: I can't do that. Read between the lines. ---snip--- So clearly Jeremy has no desire to work with Wine developers. He can't clearly demonstrate the problem. So what is there to fix? Vitaliy.
Re: Are we the reason Apple isn't helping us?
On 03/11/2012 03:10 PM, Charles Davis wrote: Sorry about the really long email, but I didn't want to flood your inboxes with 5 separate responses. On Mar 11, 2012, at 12:15 AM, Jerome Leclanche wrote: There are a thousand bugs where I would agree with you, but Jeremy's second post was I don't plan on submitting patches to you frequently enough to make it worth my time to read your policies and subscribe to your lists.. That's not trying to help, it's self-entitled behaviour. True. But my original point--that Vitaliy was unnecessarily rude to our guest--stands. You mean showing person the door when that person clearly demonstrated he does not, will not, and doesn't even have a desire to play by our rules? Every single time this ends up being a waste of time for everyone involved. And this is yet another proof of that. Jeremy: Here's a problem, and a patch to fix it. Wine developers: Follow the rules or you'll get no sympathy. Jeremy: I don't have time for that! Wine developers:We don't see any problem. Tell us more. Jeremy: I can't. Vitaliy: Then we can't take your patch, even if it is right. Jeremy: You'll have to take my patch on faith. Vitaliy: Go to hell! Screw you! I'm banning you! Jeremy: Screw you, too! I'm outta here! You better re-read the entire bug report again. What you said is offensive towards me and I request your apology! The topic is over, at least for me. You are behaving exactly the same way Jeremy did disregarding everyone and everything because _you_ perceive something differently then everyone else here. And making conclusions without reading the entire conversation or being familiar enough with bugzilla. Vitaliy.
Re: Are we the reason Apple isn't helping us?
On 03/11/2012 07:26 PM, Dan Kegel wrote: On Sun, Mar 11, 2012 at 2:42 PM, Vitaliy Margolen I guess it comes down to unnecessary roughness. In what way? Telling a user if you don't comply to _our_ rules in _our_ place you will be banned is rough? You have other ideas how to stop anyone from reopening a bug that everyone one from Wine said please do not reopen? Of course just ignoring this bug won't work, as everyone who active on bugzilla will be attempting to close it again. Sure, he was out of line, but AJ had already explained things to him. You could have let the guy get the last word in. Last time I've checked anyone can still post comments into closed bugs. So what's to fix? It's hard to quantify, but partly as a result of your comment, he seems to be actively telling folks that the Wine community is rude and unpleasant. One can never stop other people from spreading FUD if they really want to do so. we might be interested in his friends. The same idea might apply here. You think a never ending close-reopen match would have been better? And wouldn't have resulted in him being banned for real? Vitaliy.
Re: advapi32: Correct tests under WOW64 (try 2)
On 03/10/2012 12:36 AM, Alistair Leslie-Hughes wrote: Hi, Marked test as broken if it returns true. +/* WoW64 return TRUE with the LastError set */ +ok((!ret GetLastError() == ERROR_INSUFFICIENT_BUFFER) || broken(ret), %d with error %d\n, ret, GetLastError()); This is incorrect. You can check or report result of GetLastError(). But not both at the same time. You have to store the result in additional variable. Then compare and report it. Vitaliy
Re: advapi32: Correct tests under WOW64 (try 2)
On 03/10/2012 09:44 AM, André Hentschel wrote: Am 10.03.2012 17:37, schrieb Vitaliy Margolen: On 03/10/2012 12:36 AM, Alistair Leslie-Hughes wrote: Hi, Marked test as broken if it returns true. +/* WoW64 return TRUE with the LastError set */ +ok((!ret GetLastError() == ERROR_INSUFFICIENT_BUFFER) || broken(ret), %d with error %d\n, ret, GetLastError()); This is incorrect. You can check or report result of GetLastError(). But not both at the same time. You have to store the result in additional variable. Then compare and report it. Really? That's quite common practice. The value won't change between the two GetLastError() calls. It sure can change. The only thing which is bad is if you test a function inside the ok() macro and report the last error in the same ok(). Isn't that what I said? To clarify, it applies to any printf type function that has call to GetLastError() as a parameter. Vitaliy.
Re: My Idea for GSOC
On 03/07/2012 01:07 PM, Daniel Oom wrote: Hello, my name is Daniel Oom and I'm a computer science and engineering student at Chalmers in Gothenburg, Sweden. Welcome to Wine! Would, for example: fix Steam's in-game overlay feature be a good goal? Not really, as it's already working fine in most cases. You just need a proper version of gcc to compile Wine with. You can find few ideas on what to do on GSOC Wiki page: http://wiki.winehq.org/SummerOfCode Vitaliy.
Re: Users with native w2k/wxp/Vista/w7, please test (CreateTimerQueue)
On 03/06/2012 06:33 AM, joerg-cyril.hoe...@t-systems.com wrote: Hi, thank you. The 2 answers I received so far already highlight the astonishing differences among native w7 systems: Have you used timeBeginPeriod() in your tests? From long time ago Win NT supported up to 10ms call back intervals, but only guaranteed if you set time precision with timeBeginPeriod() to less then or equal to your period. Vitaliy
Re: translation: meaning of 'magazine' in winerror string
On 03/04/2012 05:32 AM, Fernando Martins wrote: the po file contains #: winerror.mc:1541 msgid The magazine is not present.\n Originally I assumed magazine was meant as a sort of storage since the typical meaning of periodical publication would not seem to make much sense in the context of a wine error. However, other translations do indeed choose the concept of periodical. Could I get an explanation of what is meant in the string? Also, this is not an isolated issue. It would be useful if screenshots of the original English dialog boxes, etc would be made available. Maybe a project for the Summer of Code? I mean, to setup an infrastructure to associate lines in po to available corresponding screenshots? Apparently this is ERROR_MAGAZINE_NOT_PRESENT/STATUS_MAGAZINE_NOT_PRESENT errors. Wine doesn't currently produce this error. Google only points to all sorts of malware when asked about this error code. Vitaliy
Re: msvcrt/file.c: file mode T (temporary file) (try 2)
On 02/25/2012 05:30 AM, Robert van Herk wrote: Try 2 fixes some blatant memory bug in the conformance test --- Windows fopen has some fancy file modes 'T' and 'D'. Currently, wine wrongly interprets 'T' as 't'. They should be interpreted differently though: 't' is text file, and 'T' is temporary file. I've fixed this, and added a conformance test. +case 'T': + FIXME(Implement T flag (temporary file, i.e. delete on close, don't flush to disk if possible)); + break; So what actually prevents you from properly implementing this? Especially that Wine already supports temporary files. Search for MSVCRT__O_TEMPORARY Vitaliy.
Re: TranslateWiki
On 02/22/2012 09:09 AM, Aric Stewart wrote: Hi, I just had a very good conversation with the mediawiki people about TranslateWiki. (http://translatewiki.net/wiki/Main_Page) There are some snags right now but they are really interested in doing work to accommodate us. We will see where that goes but so far it is looking like the most promising path. Maybe the whole problem is that we doing it wrong? Why do we need to track contribution? One of the main reasons I can thing of is LGPL constrains. What if we change license on .po files to something that is less stricter and allows easier integration with translation sites? Just my 2c. Vitaliy.
Re: kernel32: CreateMemoryResourceNotification stub should return a waitable handle
On 02/19/2012 09:26 AM, Dan Kegel wrote: Otherwise jqs spins. Fixes http://bugs.winehq.org/show_bug.cgi?id=29936 +return CreateEventA(NULL, FALSE, FALSE, Fake MemoryResourceNotification); I don't think you want a named event here. Anonymous would be better, in case the program will ask for another event. Vitaliy.
Re: [PATCH] server: Add sessions directory and 0 session symlink
On 02/16/2012 08:37 PM, Vitaliy Margolen wrote: Fixes bug 29905. --- Please ignore this. Didn't read the last bug comment. Vitaliy.
Re: winecfg: Center main window on the screen.
Please don't! I want my window manager decide based on how I configured it where to position new windows. And not every program open in the middle of the screen just because it things it's the most important thing on my screen. Vitaliy.
Re: dinput: Support INFINITE as number of effect iterations (try 2)
On 02/01/2012 11:17 AM, Marcel Hasler wrote: 2012/1/30 Marcel Haslermahas...@gmail.com: I've submitted a final patch. I don't think there's anything more I can do to improve it. Looks fine. Vitaliy.
Re: dinput: Support INFINITE as number of effect iterations (try 2)
On 01/29/2012 04:31 PM, Marcel Hasler wrote: The limit 0x7fff mentioned in input.h doesn't concern the number of iterations, just durations. I couldn't find anything on an upper limit for 'value'. I've tested 0x7fff and it works just fine. I'm afraid you'll have to dig up some documentation about what is the max value here is then. The fact that it works doesn't mean it's correct. Vitaliy.
Re: mmdevapi: Correctly convert UINT32 to INT64
On 01/30/2012 03:08 AM, joerg-cyril.hoe...@t-systems.com wrote: Michael Stefaniuc wrote: But now I see basestd.h defines LONG32. LONG32 is kinda redundant. LONG is 32bit on all Windows versions on 16bit, 32bit and 64bit Windows. Ah, thank you. However in our particular case, the value that getbuf_last is derived from is UINT32, not UINT, so for symmetry, I still recommend the explicit LONG32. UINT is the same as UNIT32: basetsd.h:84 typedef unsigned int UINT32, *PUINT32; windef.h:213 typedef unsigned intUINT, *PUINT; Vitaliy.
Re: kernel32: add a stub for LCMapStringEx (try 2)
On 01/29/2012 10:54 AM, Dmitry Timoshkov wrote: Austin Englishaustinengl...@gmail.com wrote: Forgot the include changes. Forwarding it to LCMapString would be much better. Also could we please stop adding stubs before the release? It seems every new stub dll opens up another can of worms (or should I say bugs). If we looking to stabilize Wine it's sure is the wrong way to do it. - Vitaliy
Re: dinput: Support INFINITE as number of effect iterations (try 2)
On 01/28/2012 05:56 PM, Marcel Hasler wrote: According to the DirectX reference, IDirectInputEffect::Start accepts INFINITE (aka -1) for the number of iterations. However, passing -1 to Linux via input_event results in no effect being played at all. This patch sets the number of iterations to the maximum signed value allowed if INFINITE is passed. This fixes bug 29712. This patch supersedes patch 83233. It does the same thing but adds a comment and should be easier to understand. In general looks good. Would be nice to point out that linux/input.h says that any durations should not exceed 0x7fff as a maximum value. Please don't make code easier to read by adding needless if / else. Your first patch was fine. Vitaliy.
Re: patch msvcrt.dll file.c MSVCRT_tmpfile() open_flags -- stream_flags
On 01/29/2012 07:25 AM, fred wrote: Hi Wine-Team A old bugfix on my system You need to fix at least these issues with your e-mail: - One e-mail per patch. - All patches needs to be submitted using git format-patch.. - You have to specify your full name in From: address For more details see: http://wiki.winehq.org/SubmittingPatches Vitaliy.
Re: Sound keyword on bugzilla
On 01/21/2012 10:07 AM, Jerome Leclanche wrote: I think that's the point Henri was trying to make. Most of these components are useless. Sure, you *can* pinpoint every component down, but as Henri said, if you do that, what's most likely to happen is you end up writing a patch. It's probably worth checking every category and remove the ones with less than 5-10 open bugs. First of all jerome, would you respect everyone and do a bottom posting on this mailing list. Otherwise people will just ignore everything you said. If you can't fix your mailer - use a different one. It's not an excuse to disrespect everyone else on this list. On 01/21/2012 10:01 AM, Dmitry Timoshkov wrote: Sorry, but honestly, in order to support an idea if adding/changing something in Wine bugzilla one should spend several months actively triaging bugs first. I've spent enough time doing so and can tell that you are the first person to move _all bugs_ to unknown component. And leave them there. And that all developers can't be expected to read every single bug and try to understand if it's a bug in their area or not. The whole point of triaging bugs is to collect enough data for a person(s) familiar with the general area to do a followup investigation and eventually fix the bug. Without knowing the area it's a waste of time for everyone to do investigation on _every single_ new bug. One can't go just by bug subject - in most cases it's not accurate enough or outright misleading. Same for the content, logs, etc. Vitaliy.
Re: Sound keyword on bugzilla
On 01/20/2012 12:45 PM, Austin English wrote: On Fri, Jan 20, 2012 at 11:04, Dmitry Timoshkovdmi...@baikal.ru wrote: Henri Verbeethverb...@gmail.com wrote: On 20 January 2012 17:25, Dmitry Timoshkovdmi...@baikal.ru wrote: If the problem is sound related there are usually some known words in the summary line describing the problem, why not search for them? Why do you think inventing a new keyword and adding it to the buch of bugs is easier that correctly formulate the problem using right words in the summary? Well, at least searching is easier for a well defined keyword than for a free form summary line. With a keyword you wouldn't have to take into account differences in formulation like e.g. audio vs. sound. Well, it's not that hard to have an agreement what right words to use. A person able to add a keyword also should be able to correct the summary according to established standards. That aside, it does seem to me that there's some overlap in functionality between keywords and components. I'm not quite sure how other people use the component field, but maybe we don't need both. Somewhat related, is it really useful to make the distinction between mmdevapi, winmm, dsound, etc. if it's mostly the same people working on those, and you practically need to debug a bug first before you can make that distinction anyway? It's easier for simple cases like crashes or regressions to set the component, and I agree that adding keywords duplicating components should be discouraged, and 'printing' is among of them. I'd be okay with removing printing, the component will obviously be wineps.drv or gdi32 for any printing bug (besides there aren't all that many printing bugs, and it's not seeing much activity). IMHO, it should be an additional component sound-general, or sound-unknown, or better yet unknown-sound. There are many areas like that, that require investigation by a person with knowledge of said area to properly set component. With Wine growing we can't require every developer to peruse all bugs in bugzilla all the time. And searching for some specific bugs by their subject almost never returns correct component. Case in point - right now we have 4468 bugs assigned to the unknown component. How many people actually go over those in attempt to investigate and set correct component? While keywords components overlap, more generic components will not overlap with specific ones. And if we name all of them unknown-something that will help user / bugzilla triage people to pick closer area for SMEs to do more detailed investigation. To begin with I propose to create following components: unknown-browser unknown-core unknown-d3d unknown-gui unknown-input unknown-printing unknown-sound Probably forgot a few. For triagers reassign bugs to area that looks close to the problem, unless exact issue is known. For SMEs (Subject Matter Experts) to look through bugs in their general area and reassign to more appropriate component. Vitaliy.
Re: Regarding my dinput patches
On 01/17/2012 11:19 AM, Lucas Zawacki wrote:
I want to get my dinput patches in before the code freeze, and besides
the ones that I tried to commit last week I have some patches touching
EnumDevicesBySemantics and ConfigureDevices, but I can't commit them
before the DIPROP_USERNAME and configuration file patches are in too.
Even thou we are not officially frozen I doubt AJ will accept anything
that significantly alters functionality. Unless you trying to fix some big
number of programs that don't work of course. Do you have bug # that talks
about program(s) not working?
Seeing that I'll have to change a lot of things in the patches to get
them in I want to ask some questions:
* Is %APPDATA%\DirectInput\User Maps a better place to store the
configuration files?
Much better place IMHO.
* Should I use the registry to persist the usernames in the dinput
devices instead of relying on the filename and filetimes like windows
seems to do? Where in the registry does this information belong?
Not sure what you mean here.
Just assume that Wine is being run by one user and one user only. Especially
that you storing files under a particular user's profile directory. And
since Wine doesn't allow multiple users to use the same wineprefix, it makes
sense to assume that all configuration will be generated by one user.
* Any suggestions on how to format all these W strings? I could use
just strcatW, but since I have to convert integers to strings I think
I'll have to look at sprintf variation...
Few things I can think of:
- Store all string constants as static const WCHAR fooW[] = {'f','o','o',0}.
- Just make your heap_printfW() work with provided buffer and size (both
passed by ref). And enlarge that buffer if needed. So the next call will use
the new bigger buffer. This is not speed critical so should be fine with
heap buffer. Unless of course someone else have some objections.
* I'll drop the _ in front the functions from now on. Should I send
patches to remove it in the other functions that already use it?
Yes please, but it can wait.
Vitaliy.
Re: winex11.drv: Strip the _XXX from X11DRV_GetKeyNameText when approperate
On 01/17/2012 09:32 AM, Aric Stewart wrote:
The problem is that there is no XK keycode for just Shift, Control or Menu
so X11DRV_MapVirtualKeyEx maps them to Shift_L, etc. As a result the string
XKeysymToString returns is a string with a _L or _R at the end, even if the
don't care bit is set.
+char *new_name;
+INT rc = 0;
+new_name = HeapAlloc(GetProcessHeap(),0,(idx-name)+1);
+lstrcpynA(new_name,name,(idx-name)+1);
+new_name[idx-name] = 0;
+TRACE(found scan=%04x keyc=%u keysym=%04x new_string=%s\n,
+ scanCode, keyc, (int)keys, new_name);
+if (lpBuffer nSize)
+rc = MultiByteToWideChar(CP_UNIXCP, 0, new_name, -1,
lpBuffer, nSize);
+HeapFree(GetProcessHeap(),0,new_name);
+return rc;
I don't think you need an extra buffer. Just do something like:
if (lpBuffer nSize)
{
rc = MultiByteToWideChar(CP_UNIXCP, 0, new_name, idx - name, lpBuffer,
nSize);
if (nSize idx - name)
lpBuffer[idx - name] = 0;
}
Of course need to account for rc.
Vitaliy
Re: winecfg: Add tab for most Direct3D registry settings.
On 01/15/2012 01:46 PM, Stephen Chao wrote: From my own experiences browsing most games' appdb page, everyone and their brother (including myself) tinkers with d3d registry options. That's exactly why those options where removed in the first place. To stop everyone from breaking their configuration, then report bogus bugs. Wine official stand on those type of settings; 1. Wine should detect all options automatically. And adjust them as needed (depending on a specific application and/or requested features) 2. If 1. doesn't work - file bug to have Wine fixed. PS. If you finding yourself needed to add all your custom settings back - save them as a .reg file. Then import it in one go using regedit. Vitaliy.
Re: [PATCH 1/4] server: Add mechanism to store and retrieve information about a socket (try 2).
On 01/13/2012 12:05 AM, Erich E. Hoover wrote: Real Name: Erich Hoover Changelog: server: Add mechanism to store and retrieve information about a socket. + +/* Get and set user-accessible socket variables */ +@REQ(get_socket_variable) +obj_handle_t handle;/* handle to the socket */ +unsigned int varcode; /* variable to return */ +unsigned int varsize; /* size of variable to return */ +@REPLY +char ok;/* operation status */ +int value; /* returned value of variable */ +@END + +@REQ(set_socket_variable) +obj_handle_t handle;/* handle to the socket */ +unsigned int varcode; /* variable to set */ +unsigned int varsize; /* size of variable to set */ +int value; /* value of variable to set */ +@REPLY +char ok;/* operation status */ +@END You are not using ok parameters in either of the calls. You should use status returned by wine_server_call_() for things like that. Also size seems to be redundant here. If it's an int - store it as int on both sides. This also implies you should use an array instead of struct for a storage. And drop size checks. One more thing, keep in mind that time_t is not unsigned int. But long. Vitaliy.
Re: [2/5] dinput: SetActionMap saving simple configurations to an .ini file (try 2)
On 01/10/2012 10:51 AM, Lucas Zawacki wrote:
2012/1/10 Vitaliy Margolenwine-de...@kievinfo.com:
On 01/09/2012 10:18 AM, Lucas Fialho Zawacki wrote:
From: Lucas Fialho Zawackilfzawa...@gmail.com
+static BOOL _write_private_profile_intW(const char *format, WCHAR*
section, WCHAR* key, int value, WCHAR* file)
I don't think this is such a good idea to mix ASCII and WCHAR parameters.
Even if this is for convenience in my private code? I use this a
great deal through the code.
Always converting static text from ASCII to Unicode during run-time is not a
good thing. Yes it sucks that we have to define all Unicode strings as
arrays. But such as life.
Why do you think it should be there in the first place?
Windows keeps it there, you can check it by running any application
using SetActionMap. But I suppose no application depends on it being
there specifically.
Just because Windows does something is not good enough of an excuse to do
the same. AJ said that on many occasions... Here it seems really suspicious
that any user can create files inside a program install directory structure.
And those files not even executables or libraries... If anything these files
belongs under user's profile local data files directory.
Yes, you're right. There were other mistakes as well. Is this version better?
...
while (1)
{
/* Test if it's the first time */
if (buffer == NULL)
buffer = HeapAlloc(GetProcessHeap(), 0, size*sizeof(WCHAR));
else
buffer = HeapReAlloc(GetProcessHeap(), 0, buffer,
size*sizeof(WCHAR));
Nope, you clobber original value of buffer and what you had before is
lost. So in case it fails you leaking old buffer.
However I really don't like this function, and how it's used. If all you
need is to format some strings, then you should provide your own buffer for
such a function and let it grow it if needed to. Then you save time by not
allocating / freeing same buffer multiple times.
Would you please drop leading underscore from all of your function names?
Ok. I can stop doing this from this patch onwards if you think this
convention is bad.
In *NIX world only system functions deserve leading underscore...
- Vitaliy
Re: [PATCH] programs/explorer: Fix explorer's command line parsing (try 2)
On 01/08/2012 08:45 PM, Jay Yang wrote: Fixed some small formatting issues from last time. Your patch strips spaces all over the place. For example: +LPWSTR *argv; +if(commandline[0]=='\0') Need an empty line after variable declarations. No space after if. No space around equal operator. +for(i=0;iargc;i++) No spaces after semi-colon. +copy_path_string(parameters-root,curr); No space after coma. Some other issues - you will parse over parameters given to the program. Ex: explorer /desktop=foo,800x600 program.exe /n /e Parsing inside parameters is incorrect: +LPWSTR end=strchrW(curr,','); +LPWSTR next; +if(end==NULL) +next=(end=strchrW(curr,'\0')); +else +next=end+1; ... +curr=next; You end up parsing something like this as a valid param: /n,/e,/select,c:\\ Vitaliy.
Re: [2/5] dinput: SetActionMap saving simple configurations to an .ini file (try 2)
On 01/09/2012 10:18 AM, Lucas Fialho Zawacki wrote:
From: Lucas Fialho Zawackilfzawa...@gmail.com
+static BOOL _write_private_profile_intW(const char *format, WCHAR* section,
WCHAR* key, int value, WCHAR* file)
I don't think this is such a good idea to mix ASCII and WCHAR parameters.
+static WCHAR path[] = {
+
'%','C','o','m','m','o','n','P','r','o','g','r','a','m','F','i','l','e','s','%','\\',
+'D','i','r','e','c','t','X','\\',
+'D','i','r','e','c','t','I','n','p','u','t','\\',
+'U','s','e','r',' ','M','a','p','s','\0'};
Why do you think it should be there in the first place?
+expanded_path = HeapAlloc(GetProcessHeap(), 0,
sizeof(WCHAR)*expanded_path_size);
No error checking.
+static HRESULT _save_mapping_settings(IDirectInputDevice8W *iface,
LPDIACTIONFORMATW lpdiaf, LPCWSTR lpszUsername)
+{
+static WCHAR mapexists[] = {'M','a','p','E','x','i','s','t','s','\0'};
+static WCHAR numactions[] = {'N','u','m','A','c','t','i','o','n','\0'};
Should be static const WCHAR.
+IDirectInputDevice8_GetDeviceInfo(iface,didev);
No error checking.
+va_start(args, format);
+while (1)
+{
+buffer = HeapAlloc(GetProcessHeap(), 0, size);
+if (buffer == NULL)
+break;
+n = vsnprintfW(buffer, size, formatW, args);
+if (n == -1)
+size *= 2;
+else if (n= size)
+size = n + 1;
+else
+break;
+HeapFree(GetProcessHeap(), 0, buffer);
+}
+va_end(args);
You can't do this. It seems to be you have not tested it with initial buffer
too small. You have to do va_start va_end every time you pass args to
another function.
+
+if (buffer == NULL) return NULL;
+ret = HeapReAlloc(GetProcessHeap(), 0, buffer,
sizeof(WCHAR)*(strlenW(buffer)+1) );
+if (ret == NULL) ret = buffer;
Why reallocate? It's pointless.
+extern WCHAR* _get_mapping_path(const WCHAR *device, const WCHAR *username)
DECLSPEC_HIDDEN;
Would you please drop leading underscore from all of your function names?
Vitaliy.
Re: [3/5] dinput: SetActionMap setting username
On 01/09/2012 10:18 AM, Lucas Fialho Zawacki wrote: From: Lucas Fialho Zawackilfzawa...@gmail.com +/* Set username in device */ +HeapFree(GetProcessHeap(), 0, This-username); +This-username = HeapAlloc(GetProcessHeap(), 0, sizeof(WCHAR) * (lstrlenW(username) + 1)); +lstrcpyW(This-username, username); No error checking if allocation fails. + +/* Free username */ +HeapFree(GetProcessHeap(), 0, This-username); + Comment is kind of redundant the property already called username. Vitaliy.
Re: [4/5] dinput: Device username being assigned at creation time (try 2)
On 01/09/2012 10:18 AM, Lucas Fialho Zawacki wrote: From: Lucas Fialho Zawackilfzawa...@gmail.com You leaking FindFile handle. Need to use FindClose() to close it. This is on top of more unchecked returns from HappAlloc. Vitaliy.
Re: [PATCH] programs/explorer: Fix explorer's command line parsing (try 2)
On 01/09/2012 08:34 AM, Jay Yang wrote: On 01/09/2012 09:46 AM, Vitaliy Margolen wrote: On 01/08/2012 08:45 PM, Jay Yang wrote: Some other issues - you will parse over parameters given to the program. Ex: explorer /desktop=foo,800x600 program.exe /n /e the /desktop command causes a the parsing to go back to the original command line and reparse it. I can add a comment for this. So in this particular case, manage_desktop is called with =foo,800x600 program.exe /n /e, which should be correct That's the wrong way to do it. The /desktop string can be anywhere in the command line. And you not accounting for any sort of quotes. You end up parsing something like this as a valid param: /n,/e,/select,c:\\ Windows accepts this. If one types explorer /n,/e,/select,c:\ into the windows command line, it opens My Computer with the c:\ drive selected. Incidentally this is not quite what wine does because Interesting, Windows has it worse then I thought (what a surprise). Yeah that appears to work on native. copy_path_root only tries to strip the last part of the path and the wine explorer currently doesn't actually select anything, even with the select parameter. Should I fix this in this patch or another patch? Yes separate patch please. Vitaliy
Re: DIB crash with gdb
On 12/22/2011 04:45 PM, Michael Ost wrote: Hi, I'm seeing a SEGV crash when running any wine program with wine 1.3.24 in gdb but not when running without the debugger. The crash is happening when writing to memory allocated by CreateDIBSection in the function create_alpha_bitmap(). The code is in user32/cursoricon.c. This is not a crash, this is the way it supposed to work. At least until DIB is finished and this hack isn't needed. Wine uses this trick to detect when DIB memory needs to be synchronized with the X server. Vitaliy.
Re: Would making the Wine file dialogs know about the Nautilus bookmarks be a reasonable thing?
On 12/12/2011 04:34 PM, Scott Ritchie wrote: Would it be appropriate to display those self-added bookmarks in Wine? I'm worried there might be something I'm missing about the implementation that would make this difficult or a bad idea. It would only work with v6 controls, not the old dialogs Wine currently have. There is no place to add such bookmarks. Vitaliy.
Re: dinput: Clarify a translatable string
On 12/04/2011 01:33 AM, Frédéric Delanoy wrote: me Could you explain what that Sort Assigned means? author I was planning on sorting the button/action mappings putting the ones which already have an assigned mapping first, and then all the others. --- +AUTOCHECKBOXSort Assigned Mappings, IDC_CHECKBOXSORT, 120, 215, 90, 8 A better way of saying this would be Show assigned first. Vitaliy.
Re: [2/2] winecfg: Bump NT 3.51 to Service Pack 5
On 12/01/2011 12:15 PM, André Hentschel wrote:
---
-{ nt351, Windows NT 3.5,3, 51, 0x421, VER_PLATFORM_WIN32_NT, Service Pack
2, 0, 0, WinNT},
+{ nt351, Windows NT 3.51, 3, 51, 0x421, VER_PLATFORM_WIN32_NT, Service Pack
5, 5, 0, WinNT},
You need to do the same in ntdll/version.c
Vitaliy.
Re: Compiling Wine without prelink
On 11/27/2011 03:24 PM, Roger Cruz wrote: Could you expand what exactly would not work? Some copy-protection systems, and any other programs that do dlls patching using IPC. They assume that some system dlls (namely ntdll, kernel32, and user32) are always loaded to the same address for every system process. It was initially added to support SafeDisk copy protection. Now it's hard to tell what other programs might depend on this. If you do not have prelink, Wine will not be able to load these DLLs at the same address and some programs will fail in mysterious ways. -Vitaliy
Re: [PATCH 1/5] user32/tests: Initial tests for RawInput functions.
On 11/27/2011 09:10 PM, Marvin wrote: 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=15663 Your paranoid android. === WVISTAADM (32 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 === W2K8SE (32 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 === W7PRO (32 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 === W7PROX64 (32 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 === TEST64_W7SP1 (32 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 === W7PROX64 (64 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 === TEST64_W7SP1 (64 bit input) === input.c:1807: Test failed: GetRawInputDeviceInfoA failed: -1, 1024, 87 Interesting. This works on Win XP... Will modify the test. BTW anyone can approve my account request to test bot? Vitaliy.
Re: dinput: Stub IDirectInputJoyConfig8 interface. (try 2)
On 11/25/2011 04:14 AM, Jacek Caban wrote:
Hi Vitaliy,
On 11/25/11 04:17, Vitaliy Margolen wrote:
Fix QueryInterface, simplify use of AddRef, and replace
LPDIRECTINPUTJOYCONFIG8
with IDirectInputJoyConfig8 *.
---
dlls/dinput/dinput_main.c | 7 ++
dlls/dinput/dinput_private.h | 2 +
dlls/dinput/joystick.c | 196 ++
include/dinputd.h | 243 ++
4 files changed, 448 insertions(+), 0 deletions(-)
create mode 100644 include/dinputd.h
+ if (IsEqualGUID(IID_IDirectInputJoyConfig8, riid ))
+ {
+ return create_joy_config( riid, ppobj );
+ }
This has the same problem as we discussed about msxml3 lately:
http://www.winehq.org/pipermail/wine-devel/2011-November/093279.html
Are you sure native is broken the same way?
You were right. It is a part of IDirectInput not a standalone class. Sent a
fixed patch.
Vitaliy.
Re: dinput: Stub IDirectInputJoyConfig8 interface.
On 11/24/2011 12:27 PM, Michael Stefaniuc wrote: Hello Vitaliy, On 11/24/2011 06:28 PM, Vitaliy Margolen wrote: --- dlls/dinput/dinput_main.c|7 ++ dlls/dinput/dinput_private.h |2 + dlls/dinput/joystick.c | 195 + include/dinputd.h| 243 ++ 4 files changed, 447 insertions(+), 0 deletions(-) create mode 100644 include/dinputd.h we are trying to get rid off the LPJUNK, IDirectInputJoyConfig8 * is so much more nicer than LPDIRECTINPUTJOYCONFIG8. IMHO - it's not. And it deviates from the PSDK headers, and the rest of the dinput. Not even saying that headers won't match declaration. While the above is more of a nit-pick (and an easy search and replace patch for me) the QueryInterface implementation is not correct: - Query for IID_IUnknown is not supported. You right, will fix this. - Use IDirectInputJoyConfig8_AddRef() on the interface without the cast Will change that as well. Vitaliy.
Re: [3/4] winemaker: Be less picky when detecting the target type (try 2)
On 11/19/2011 11:42 AM, André Hentschel wrote:
this catches more than one 0 after the space and possible 0s after the x
+if (/[[:space:]]0+x0*101$/) {
For more then one you should use + not *. * means any number, including 0.
Vitaliy
Re: dinput: Delete the static critical section when unloading the dll.
On 11/16/2011 08:31 AM, Francois Gouget wrote: --- I moved DllMain() to the end of the file so it comes after the variable declaration. It's already forward declared few lines after DllMain(). So you should move that declaration instead. Hm... it seems this patch already committed. Alexadre, would you please give people at least 24 hours to review all patches sent to wine-patches before committing them? Vitaliy.
Re: dinput: Delete the static critical section when unloading the dll.
On 11/16/2011 07:34 PM, Vitaliy Margolen wrote: On 11/16/2011 08:31 AM, Francois Gouget wrote: --- I moved DllMain() to the end of the file so it comes after the variable declaration. It's already forward declared few lines after DllMain(). So you should move that declaration instead. Hm... it seems this patch already committed. Alexadre, would you please give people at least 24 hours to review all patches sent to wine-patches before committing them? On the side note, why do we need to delete statically allocated critical section in the first place? Vitaliy.
Re: winemaker: Be less picky when detecting the target type
On 11/14/2011 04:04 AM, Francois Gouget wrote:
On Fri, 11 Nov 2011, André Hentschel wrote:
seen some broken project files which still worked with msvc but not with
winemaker
---
tools/winemaker |8
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/winemaker b/tools/winemaker
index 2ee79ea..921f8ef 100755
--- a/tools/winemaker
+++ b/tools/winemaker
@@ -589,16 +589,16 @@ sub source_scan_project_file($$$)
#print $prj_name;
next;
} elsif (/^# TARGTYPE/) {
-if (/[[:space:]]0x0101$/) {
+if (/0x.*101$/) {
The various .* worry me a bit. Would something like [0-9a-fA-F]* be
permissive enough?
I think he was trying to match x too. At a very minimum the match needs to
be less greedy (.*?).
Vitaliy.
Re: [1/2] kernel32/tests: add tests for GetDllDirectoryA
On 11/06/2011 09:30 AM, Thomas Faber wrote: Hey, On 2011-11-06 17:25, Vincent Povirk wrote: I don't think you can rely on C:\Windows existing. indeed. :) But the function only requires valid path syntax, not a path that actually exists. Then you should be explicit about it with a path like c:\Some\Valid\Path. Then people won't have questions. Vitaliy.
Re: [1/2] kernel32/tests: add tests for GetDllDirectoryA
On 11/06/2011 09:14 AM, Thomas Faber wrote:
+length = strlen(dll_directories[i]);
+assert(sizeof(buffer) length + 1);
Please don't do this. Tests shouldn't assert because programmer failed to
pick the right buffer size of hard-coded strings.
+ok(GetLastError() == 0xdeadbeef, Error is %u\n, GetLastError());
You should get value of GetLastError() before the ok check here. See all
places that check values of GetLastError().
+ok(buffer[length + 1] == 'A', i=%d, Buffer overflow\n, i);
+ok(buffer[ret] == 0, i=%d, Wrong length or not null terminated\n, i);
+ok(strcmp(buffer, dll_directories[i]) == 0, i=%d, Wrong path returned:
'%s'\n, i, buffer);
You can do all these with one memcmp call.
+ok(buffer[length + 1] == 'A', i=%d, Buffer overflow\n, i);
+ok(buffer[ret] == 0, i=%d, Wrong length or not null terminated\n, i);
+for (j = 0; buffer[j]; j++)
+if (buffer[j] != dll_directories[i][j])
+{
+ok(0, i=%d, Wrong path returned: %s\n, i,
wine_dbgstr_w(buffer));
+break;
+}
Same for this.
+ok(buffer[0] == 0 || /* XP, 2003 */
+ buffer[0] == 'A', i=%d, Buffer overflow\n, i);
Please mark bad case with broke() to check for correct Wine behavior.
Also please combine A W tests. They do mostly the same things.
Vitaliy.
Re: MS XInput*.dll implementation
I've got a general question about the Windows subsystem I'm a bit interested of and considering spending some time trying to implement it. What I'm talking about is new input handling API MS positions as the DirectInput successor: XInput 1.x. If you really want it to get in just do it via DI. Use old patches and projects like x360ce for ideas, code, and documentation. Too bad x360ce is in c#. Implementing XInput properly via X11 is impossible. X11 does not support force-feedback joysticks. One can wait for another 5 years for broken interface. And another 5 to get it fixed. Vitaliy.
Re: [3/6] dinput: SetActionMap saving simple configurations to an .ini file
Few nit picks.
On 10/22/2011 02:53 PM, Lucas Fialho Zawacki wrote:
+static void _write_private_profile_intW(const char *format, WCHAR* section,
WCHAR* key, int value, WCHAR* file)
+{
+WCHAR* value_str = heap_printfW(format, value);
Here and everywhere else you not checking if memory allocation succeeded.
+WCHAR path[] = {
+
'%','C','o','m','m','o','n','P','r','o','g','r','a','m','F','i','l','e','s','%','\\',
+'D','i','r','e','c','t','X','\\',
+'D','i','r','e','c','t','I','n','p','u','t','\\',
+'U','s','e','r',' ','M','a','p','s','\0'};
Missing static const qualifiers. Also the above directory does not exists
and you not explicitly creating it.
+guid_strA = debugstr_guid(lpdiaf-guidActionMap);
+guid_size = MultiByteToWideChar(CP_ACP, 0, guid_strA, -1, NULL, 0);
I'm not sure debug function is ok to be used in the regular code. Also, if
need it as unicode why not print it as unicode from the beginning?
+IDirectInputDevice_GetDeviceInfo(This-IDirectInputDevice8W_iface,
didevi);
You should use IDirectInputDevice8W_from_impl() function instead of directly
accessing internals.
+memcpy(username, find_data.cFileName, (end -
find_data.cFileName + 1) * sizeof(WCHAR));
No checking for big enough buffer. Assuming that file names are in specified
format.
Also I'm not exactly sure why do you need a mapping user name anyway? Wine
does not support multiple users. The user currently running Wine would be
the one created all the files. All this seems to be redundant.
Vitaliy.
Re: 80186: [PATCH 2/2] dinput: Restore checks for DirectX 3 buffer size.
Could be because of definition problems. Here's what I get compiling wine64: ../../../../wine.git/dlls/dinput/tests/device.c:117:5: warning: comparison is always false due to limited range of data type ../../../../wine.git/dlls/dinput/tests/device.c:120:5: warning: comparison is always false due to limited range of data type HRESULT defined as typedef LONG HRESULT;. But error code defined as: #define DIERR_NOTBUFFERED 0x80040207L Not sure what to do here. Test passed on all win64 bit platforms according to test bot. So unless we want to redefine all LONG constants in PSDK (and remove that last L) I'd say ignore this. Vitaliy. On 10/20/2011 09:46 AM, Dan Kegel wrote: Hi Vitaliy, could you check on win64? I had a test failure there with your patch. - Dan On Thu, Oct 20, 2011 at 7:49 AM,build...@kegel.com wrote: This is an experimental automated build and test service. Please feel free to ignore this email while we work the kinks out. For more info about this message, see http://wiki.winehq.org/BuildBot The Buildbot has detected a failed build on builder runtests-default-x86_64 while building Wine. Full details are available at: http://buildbot.kegel.com/builders/runtests-default-x86_64/builds/769 (though maybe not for long, as I'm still reinstalling the buildbot periodically while experimenting) BUILD FAILED: failed shell_3 Errors: alarum: failed command was ../../../wine dinput_test.exe.so device.c device.c:117: Test failed: GetDeviceData() should have failed: 80040207 device.c:120: Test failed: GetDeviceData() should have failed: 80040207 device.c:117: Test failed: GetDeviceData() should have failed: 80040207 device.c:120: Test failed: GetDeviceData() should have failed: 80040207 device.c:117: Test failed: GetDeviceData() should have failed: 80040207 device.c:120: Test failed: GetDeviceData() should have failed: 80040207 device.c:117: Test failed: GetDeviceData() should have failed: 80040207 device.c:120: Test failed: GetDeviceData() should have failed: 80040207 make: *** [device.ok] Error 8
Re: 80186: [PATCH 2/2] dinput: Restore checks for DirectX 3 buffer size.
On 10/20/2011 06:18 PM, Dan Kegel wrote: On Thu, Oct 20, 2011 at 5:15 PM, Vitaliy Margolen wine-de...@kievinfo.com wrote: device.c:117: Test failed: GetDeviceData() should have failed: 80040207 device.c:120: Test failed: GetDeviceData() should have failed: 80040207 make: *** [device.ok] Error 8 Could be because of definition problems. Here's what I get compiling wine64: ../../../../wine.git/dlls/dinput/tests/device.c:117:5: warning: comparison is always false due to limited range of data type ../../../../wine.git/dlls/dinput/tests/device.c:120:5: warning: comparison is always false due to limited range of data type HRESULT defined as typedef LONG HRESULT;. But error code defined as: #define DIERR_NOTBUFFERED 0x80040207L Not sure what to do here. Test passed on all win64 bit platforms according to test bot. So unless we want to redefine all LONG constants in PSDK (and remove that last L) I'd say ignore this. How can we ignore it? Once we check it in, the 64 bit build will start failing its tests :-( So you saying we should modify all long error constants in PSDK? I'm not sure what else can be done. Vitaliy.
Re: 80186: [PATCH 2/2] dinput: Restore checks for DirectX 3 buffer size.
On 10/20/2011 06:26 PM, Vitaliy Margolen wrote: On 10/20/2011 06:18 PM, Dan Kegel wrote: On Thu, Oct 20, 2011 at 5:15 PM, Vitaliy Margolen wine-de...@kievinfo.com wrote: device.c:117: Test failed: GetDeviceData() should have failed: 80040207 device.c:120: Test failed: GetDeviceData() should have failed: 80040207 make: *** [device.ok] Error 8 Could be because of definition problems. Here's what I get compiling wine64: ../../../../wine.git/dlls/dinput/tests/device.c:117:5: warning: comparison is always false due to limited range of data type ../../../../wine.git/dlls/dinput/tests/device.c:120:5: warning: comparison is always false due to limited range of data type HRESULT defined as typedef LONG HRESULT;. But error code defined as: #define DIERR_NOTBUFFERED 0x80040207L Not sure what to do here. Test passed on all win64 bit platforms according to test bot. So unless we want to redefine all LONG constants in PSDK (and remove that last L) I'd say ignore this. How can we ignore it? Once we check it in, the 64 bit build will start failing its tests :-( So you saying we should modify all long error constants in PSDK? I'm not sure what else can be done. Ok, casting error code to HRESULT seemed to work. Vitaliy.
Re: Anyone wants to ban spammer on forum
On 10/19/2011 09:49 AM, Jeremy Newman wrote: On 10/19/2011 10:21 AM, Tijl Coosemans wrote: The past few days there's been spam messages every few minutes and as soon as you ban an account an other one appears. Obviously the bots have figured out the answer to the captcha. Alrighty then, time for me to change the question... Done! The new question is: What operating system does Wine run applications from? Should be obvious for a human. Hope that won't upset users of other well known OS g. Any ETA on the new version of PHPBB? Vitaliy.
Anyone wants to ban spammer on forum
So any admins actually watching and want to bad roberbdib3a on forum? Also why aren't every moderator has these rights to block spammers, since we have only one forum. - Vitaliy
Re: Anyone wants to ban spammer on forum
On 10/18/2011 07:37 PM, Vitaliy Margolen wrote: So any admins actually watching and want to bad roberbdib3a on forum? Also why aren't every moderator has these rights to block spammers, since we have only one forum. So no takers? I'm guessing we need more forum admins then, so more timezones can be covered. Vitaliy.
Re: winecfg: Remove driver selection from Audio tab.
On 09/29/2011 02:11 AM, Alexandre Julliard wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: The part you forgot is Linux (referring to kernel here) being the most successful FOSS project, used on super computers, stock exchanges, banks, etc. While Wine still in it's perpetual alpha-beta state, limited user base, and not recommended for production use. Not saying that AJ's ways are all that bad, but obviously not giving the same results. Don't you think it's time to change things up a bit? So what do you suggest? I can think of few things can be implemented right away and see how it goes: 1. Longer release cycle. Lets start with 3-4 weeks and extend if needed. Allow any new features first week of the release cycle only. Next two weeks are bug fixes and stabilization before release. Or have same 2 week release cycle with every other release being release candidate and code freeze for the second 2 weeks to stabilize it. 2. All major changes (substantial code rewrites, new dlls (excluding stubs), core dll / server changes) should be submitted as a whole series. Alternatively available as an external tree. To not completely stop development of major features it should be ok for small trickle of changes like DIB which one by one replacing existing functionality. While still requiring same level of testing. Such changes should have complete design proposal sent to wine-devel for discussions, preferably prior to codding. Don't need diagrams but at a minimum explanation what is the plan, how it supposed to work, what's wrong with current code that can't be fixed. To not put all weight lifting on one person, person(s) most familiar with the area _have_ to review and approve before continuing. Even tho we don't have official maintainers, this might be the time to assign this responsibility. Example of major changes: dsound rewrites, substantial d3d changes, wineserver/ntdll/kernel/user. 3. All regressions should be treated as blockers. And fixed before next release. If a regression is in some other area then patch introducing regression, no patches should be accepted from anyone else to the area in question until this regression is fixed. Or a work around found. Of course this is just a suggested plan not set in stone. Just wanted to bring this up before Wineconf so people can think about it. Can continue the conversation there as well. -Vitaliy
Re: winecfg: Remove driver selection from Audio tab.
On 09/28/2011 04:18 AM, Alex Bradbury wrote: Do correct me if I'm wrong here, but users who don't want regressions in their favourite apps/games should be using the stable release. It doesn't seem fair to complain about regressions being ignored unless 1.4 releases with a significant number of them. If Wine would release stable versions every 3-4 months sure. But last stable version released on April 8. And only contains small number of fixes since wine-1.2 which was released on July 16. Many programs don't work with wine-1.2.3 for number of reasons. Besides, everywhere (forum, bugzilla, irc) we tell users to upgrade to latest development version, because we not going to fix any bugs in old stable versions. -Vitaliy
Re: winecfg: Remove driver selection from Audio tab.
On 09/28/2011 05:31 AM, Michael Stefaniuc wrote: Vitaliy Margolen wrote: On 09/27/2011 12:17 PM, Michael Stefaniuc wrote: first place. It would be much better to see the entire conversion to mmdevapi in a separate tree stabilized before making it's way to main tree. Like in a development tree? The Wine 1.3.x tree is a development tree. Also how many people would you think would use that extra mmdevapi tree? One? Two? So you'll get the same untested code merged at a later time. I'd expect most developers to test it it. Or at least those who interested in the specific area. Some users, especially those who reported errors about sound would be pointed to that tree as well. So it will get testing. Wine is driven by wrong objectives. Most developers here don't give much rip about regressions if the code they've put in is right. This is Yes, sadly there are Wine developers that consider regressions they have introduced just normal bugs which they will fix eventually if they are My suggestion would be to revert those fixes unless said developers willing to fix what they broke. Linus was pretty clear that he won't accept patches from such developers. Complete 180 from Alexandre's view. interested. But there are a lot of other developers that fix the regressions they have introduced with a hight priority. I venture to say that most Wine developers prioritize regression bugs above normal bugs. Unfortunately no one goes back and fixes those other regressions. Many users quit using Wine because of them. Not because of all other great stuff was added to Wine. And no, Andrew's code is *NOT* right by any means! If we preferred the right code we would have taken Maarten's code which nuked dsound out More so reasons to have a separate tree to stabilize it before merging with main line. Again this is an 1:1 backend replacement with feature parity on the I think not! Andrew's biggest desire was to drop hardware acceleration support. The only part that actually did work and produced best sound. I'd like you guys to think about what really is important users or code? Users. But you seem to prefer the old code... I prefer working code, regardless of it's age. What I really like to see is stable Wine versions being released more often if you think that the development tree is the right place to test nuclear options... - Vitaliy
Re: winecfg: Remove driver selection from Audio tab.
On 09/28/2011 11:04 AM, André Hentschel wrote: Am 28.09.2011 15:09, schrieb Vitaliy Margolen: On 09/28/2011 05:31 AM, Michael Stefaniuc wrote: Vitaliy Margolen wrote: On 09/27/2011 12:17 PM, Michael Stefaniuc wrote: Wine is driven by wrong objectives. Most developers here don't give much rip about regressions if the code they've put in is right. This is Yes, sadly there are Wine developers that consider regressions they have introduced just normal bugs which they will fix eventually if they are My suggestion would be to revert those fixes unless said developers willing to fix what they broke. Linus was pretty clear that he won't accept patches from such developers. Complete 180 from Alexandre's view. Still Linus isn't our projectleader, but AJ. And AJ was pretty clear he won't do reverts everytime somethings wrong, but will look for a fix. And AJ does his job for Wine nearly as long as Linus does for Linux and look where Wine is today, so i don't see the point for quoting Linus in that discussion. The part you forgot is Linux (referring to kernel here) being the most successful FOSS project, used on super computers, stock exchanges, banks, etc. While Wine still in it's perpetual alpha-beta state, limited user base, and not recommended for production use. Not saying that AJ's ways are all that bad, but obviously not giving the same results. Don't you think it's time to change things up a bit? - Vitaliy
Re: winecfg: Remove driver selection from Audio tab.
On 09/27/2011 12:17 PM, Michael Stefaniuc wrote: On 09/27/2011 03:20 AM, Vitaliy Margolen wrote: Could we please wait another week (until after the wineconf) before doing these whole sale changes? Please leave sound system and related winecfg code as-is. Why? Come to think of it this would be the right time to do it: - We are at the start of the commit period for 1.3.30 - People will be busy traveling to/from WineConf so a regression won't halt their work. - Andrew won't travel so he can fix regressions. - We can directly motivate Andrew to fix those regressions Oh, regressions still open? No dinner for you tonight!. Exactly because of all the regressions introduced with such a huge change. I'm not a big fan of ripping things out and replacing them with new and shiny just because no one understands how it all needs to be done in the first place. It would be much better to see the entire conversion to mmdevapi in a separate tree stabilized before making it's way to main tree. Wine is driven by wrong objectives. Most developers here don't give much rip about regressions if the code they've put in is right. This is exactly the wrong way to do it. Don't take it from me, take it from the person who knows it much better and knows what he is talking about: http://h30565.www3.hp.com/t5/Feature-Articles/Linus-Torvalds-s-Lessons-on-Software-Development-Management/ba-p/440 [T]he thing that really matters is the users of the code. The code itself is unimportant; the project is only as useful as people actually find it. You think that all people who'll find their program broken because of this sound rewrite care about the code? Or that now dsound talks to mmdevapi instead of winmm? Or remarks by Andrew that he doesn't want to fix old code because it's going away anyway. And he'd rather fix new code. a good excuse for their game to stop working on Wine? I'd like you guys to think about what really is important users or code? - Vitaliy
Re: winecfg: Remove driver selection from Audio tab.
On 09/26/2011 09:10 AM, Andrew Eikum wrote: On Mon, Sep 26, 2011 at 04:50:22PM +0200, joerg-cyril.hoe...@t-systems.com wrote: I'm not satisfied with the removal of the audio tree from winecfg. Beside allowing to switch or disable drivers, one key feature was to allow inspecting the devices that winmm found on this system. The goal of that patch, like the subject says, was to remove driver selection. Obviously a side-effect of removing that entire tree was removing the list of detected devices. I personally don't really see the use of listing the detected devices, which is why I just removed the tree wholesale. If folks find the device list useful, I wouldn't be at all opposed to adding it again in some form (obviously it would use MMDevAPI directly instead of WinMM like the old code... except for MIDI devices, I guess). I am opposed to adding the driver selection functionality back in, though. Andrew Could we please wait another week (until after the wineconf) before doing these whole sale changes? Please leave sound system and related winecfg code as-is. Vitaliy.
Re: ntdll: Fix two buffer overflow conditions in RtlDosPathNameToNtPathName_U.
On 09/06/2011 08:46 AM, Octavian Voicu wrote: On Tue, Sep 6, 2011 at 4:40 PM, Vitaliy Margolen wine-de...@kievinfo.com wrote: The fix is not entirely correct. UNICODE_STRING does not have to have a terminating \0 character. The code should not use str* functions on not zero-terminated strings. I was also unsure about this when coding the patch, but I read here [1] that: Specifies the length, in bytes, of the string pointed to by the Buffer member, not including the terminating NULL character, if any. So it's not a mistake to include a NULL character. I figured it's not worth the trouble to change the last strcpy into memcpy, especially because it could also be the case that it would introduce regressions (in case some other code wrongly depends on that NULL). Should I change offending strcpyW to memcpy? Some ntdll functions do put terminating \0 character into UNICODE_STRINGs. You can write a test to see if RtlDosPathNameToNtPathName_U is one of them. But by definition U_S does not require terminating \0. And many places don't put it there. Regardless, last strcpy strlen should go and be replaced with memcpy pointer arithmetic. Vitaliy.
Re: ntdll: Fix two buffer overflow conditions in RtlDosPathNameToNtPathName_U.
On 09/06/2011 06:23 AM, Octavian Voicu wrote:
First one is an off-by-one error: RtlGetFullPathName_U, on success, returns
the number of bytes written, without counting the terminating NULL. The
allocated size for ntpath-Buffer didn't account for that NULL byte, so
for UNC paths the NULL byte would be overflown.
Second one is caused by an USHORT overflow in ntpath-MaximumLength.
Calling the function with a path longer than 65535-8 bytes would allocate
a much shorter buffer and lead to a buffer overflow.
--
Steps to reproduce second issue are described here:
http://packetstormsecurity.org/files/view/76170/wine-overflow.txt
Funny fact: Windows had the same USHORT overflow bug:
http://www.securityfocus.com/archive/1/323508
---
dlls/ntdll/path.c |8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/dlls/ntdll/path.c b/dlls/ntdll/path.c
index 3207720..6138fa8 100644
--- a/dlls/ntdll/path.c
+++ b/dlls/ntdll/path.c
@@ -383,8 +383,14 @@ BOOLEAN WINAPI RtlDosPathNameToNtPathName_U(PCWSTR
dos_path,
if (!(ptr = RtlAllocateHeap(GetProcessHeap(), 0, sz))) return FALSE;
sz = RtlGetFullPathName_U(dos_path, sz, ptr, file_part);
}
+sz += (1 /* NUL */ + 4 /* unc\ */ + 4 /* \??\ */) * sizeof(WCHAR);
+if (sz MAXWORD)
+{
+if (ptr != local) RtlFreeHeap(GetProcessHeap(), 0, ptr);
+return FALSE;
+}
-ntpath-MaximumLength = sz + (4 /* unc\ */ + 4 /* \??\ */) * sizeof(WCHAR);
+ntpath-MaximumLength = sz;
ntpath-Buffer = RtlAllocateHeap(GetProcessHeap(), 0,
ntpath-MaximumLength);
if (!ntpath-Buffer)
{
The fix is not entirely correct. UNICODE_STRING does not have to have a
terminating \0 character. The code should not use str* functions on not
zero-terminated strings.
Vitaliy
Re: Undocumented registry settings
On 09/05/2011 08:59 AM, Henri Verbeet wrote: On 4 September 2011 21:20, Vitaliy Margolenwine-de...@kievinfo.com wrote: Would you mind documenting recently added registry entries to wined3d on Wiki page http://wiki.winehq.org/UsefulRegistryKeys ? I can't find description of AlwaysOffscreen. I updated the page a bit. Awesome, thank you! Vitaliy.
Undocumented registry settings
Henri, Would you mind documenting recently added registry entries to wined3d on Wiki page http://wiki.winehq.org/UsefulRegistryKeys ? I can't find description of AlwaysOffscreen. Vitaliy.
Re: Questions on WINE releases and stability
On 07/25/2011 11:54 AM, lo...@museresearch.com wrote: - Does the development branch of WINE go through a battery of tests before it is posted? What kind of criteria has to be met before a development release is made available on the main page? Each applied patch have to compile and pass all conformance tests at least on Alexandre's PC. If even a single test fails, patch won't be applied. Alexandre usually picking most dramatic changes at the beginning of each release cycle for development version (2 weeks). Gradually lowering scale of changes towards the release date (every other Friday). - It's my understanding that the stable branch only has select features and patches ported over (from the wiki: ..with only minimal changes merged in). Would this mean that if I compared a stable release with a development release from around the same time that the two releases would have significant differences? Specifically, I'm wondering about 1.2.3 vs. 1.3.15. The Wine development version wine-1.1.44 eventually became Wine-1.2 release. The goal of the release was to stabilize Wine (no new features where accepted for a long time, only bug fixes). And to fix number of major bugs targeted for wine-1.2 (see bugzilla for details). Wine-1.2.x are minor releases with few major show-stopper bug fixes back-ported from the development tree. Wine-1.2.1 also has number of translation additions and fixes. As well as some minor usability improvements to winecfg. - Is 64-bit support in version 1.2.3 generally usable and stable, or is No, it's not suitable for general use. It was one of the main targets for wine-1.2 release but had lots of issues for more complex applications. the current development release a lot more mature in that respect? Yes. Current development version should be able to run majority of simple 64-bit applications. And some big 64-bit applications with some issues. Vitaliy.
Re: [PATCH 1/3] wineoss.drv: Attempt to verify that the system is actually running OSS
On 07/22/2011 08:35 AM, Andrew Eikum wrote: This is really crude, but does seem to work. I tested it correct with just ALSA 1.0.24; ALSA 1.0.24's snd-pcm-oss and snd-pcm-mixer modules; and on OSS4.2. It was also tested by Gerald Pfeifer on FreeBSD 8.2 and 9.0. This lets us attempt to load the OSS driver, and if that fails, then try the ALSA driver. Automatic driver selection! You forgetting, that users can select multiple drivers in winecfg now. And that's what some people have in their registry. The actual approach is seems fishy to me as well. The reason ALSA driver is 1st is because most systems has it as the main sound subsystem. Putting something [oss] first in the list, and failing if it wasn't explicitly asked for defeats the whole idea about something being default, and having alternate fallback(s). With same logic one would add all other sound drivers and make them fail, unless explicitly asked for. Vitaliy.
Re: [4/5] dinput: Added device's owner username set by SetActionMap and the respective GetProperty to retrieve it
On 07/08/2011 12:01 PM, Lucas Fialho Zawacki wrote: +memcpy(ps-wsz, This-owner, strlenW(This-owner)*sizeof(WCHAR)); You can use strcpyW instead. Or even lstrcpynW to guard against buffer overflow. @@ -1459,10 +1469,22 @@ HRESULT WINAPI IDirectInputDevice8WImpl_SetActionMap(LPDIRECTINPUTDEVICE8W iface LPCWSTR lpszUserName, DWORD dwFlags) You need to modify ascii version of SetActionMap as well. Vitaliy.
Re: [1/10] dinput: Implement IDirectInput::RunControlPanel.
On 07/05/2011 06:18 AM, Andrew Nguyen wrote: --- dlls/dinput/dinput_main.c | 15 - dlls/dinput/tests/Makefile.in |1 + dlls/dinput/tests/dinput.c| 69 + 3 files changed, 84 insertions(+), 1 deletions(-) create mode 100644 dlls/dinput/tests/dinput.c Patches look good to me. Vitaliy.
Re: [PATCH] ntdll/tests: Added tests for ObjectTypeInformation class in NtQueryObject
On 07/05/2011 07:20 PM, Austin Lund wrote: +todo_wine ok( len= sizeof(UNICODE_STRING) + 88 + sizeof(type_event) + sizeof(WCHAR), unexpected len %u\n, len ); What is that magic number 88? If it's a size of some structure - then please use it explicitly. Vitaliy.
Re: [PATCH 1/3] user32/tests: Added client-side raw input function tests (try 5 resend)
On 06/26/2011 08:49 AM, André Hentschel wrote: Am 22.06.2011 06:26, schrieb Vincas Miliūnas: +if (pDefRawInputProc pGetRawInputBuffer pGetRawInputData pGetRawInputDeviceList +pGetRawInputDeviceInfoA pGetRawInputDeviceInfoW pGetRegisteredRawInputDevices pRegisterRawInputDevices) You should avoid such a construct, move the check to test_functions that actually need these functions. e.g. check for pGetRawInputBuffer only in test_get_raw_input_buffer i didn't looked very close, but if the tests depend on each other there should be a comment. the rest of that patch looks fine to me I don't see a point here. It's all or nothing. There is only one raw input version, that is implemented starting from WinXP. All those functions are required. Vitaliy.
Re: GSoC: dinput8 Action Mapping
On 06/24/2011 11:10 PM, Lucas Zawacki wrote: As a hello word programmer, i can't judge the code quality, but i can say, that it worked nearly flawlessly. I was able to drive/play on keyboard :-D That's very nice to hear :) The only issue i found in NFS's keyboard configuration: * When i press PAUSE KEY, then NFS says Num_Lock * When i press NUMLOCK KEY, then NFS says Pause Hmm, actually I construct the mapping based on the positions of the objects in the keyboard dataformat and PAUSE and NUM LOCK seem to be in different positions in native and wine. Attached are two dumps of the keyboard objects with builtin and native dinput for comparison. This is not related to data format. But auto-generated events in x11drv to turn on/off num/lock. Apparently native dinput has a mechanism to ignore these events. Vitaliy.
Re: Disablin xinput2 for now?
On 06/20/2011 02:55 AM, Marcus Meissner wrote: As this might be a X.org issue, should we packagers turn off xinput2 support for the time being? If that will fix all dinput regressions - absolutely. Or add a registry option to disable xinput2. Vitaliy.
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 06/20/2011 05:28 AM, Austin Lund wrote: Thanks for the review. Does this patch address these concerns correctly? No quite there yet: +SERVER_START_REQ( create_snapshot ) You not checking if this call succeeded or not. +while (sizeof(ULONG) + sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count = len) You don't need a loop to calculate new size. +shi = RtlReAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, shi, len); +if (shi == NULL) +return STATUS_NO_MEMORY; You leaking old shi here. Also why do you need to zero allocated memory if you assigning all of it valid values? Vitaliy.
Re: [PATCH 1/3] user32/tests: Added client-side raw input function tests (try 4)
On 06/20/2011 02:55 AM, Vincas Miliūnas wrote: Fixed function name typo, compiler warnings, code style issues in a fragment of new code and incorrect usage of wcslen in the tests instead of lstrlenW after previous changes. My apologies :). +#define DEADBEEF 0xdeadbeef Please don't do this. Just explicitly use 0xdeadbeef everywhere. +todo_wine ok(ret == 0 GetLastError() == DEADBEEF count = 1, Given (NULL, count, sizeof), +GetRawInputDeviceList should return the number of raw input devices\n); If call succeeded you don't need to check last error. +device_list = malloc(count * sizeof(RAWINPUTDEVICELIST)); Don't use malloc/free in Wine. Use HeapAlloc / HeapFree. +ret = pGetRawInputDeviceList(device_list, count2, sizeof(RAWINPUTDEVICELIST)); +todo_wine ok(ret == (UINT)-1 GetLastError() == ERROR_INSUFFICIENT_BUFFER count2 == count, +Given (device_list, count2 = 0, sizeof), GetRawInputDeviceList should return an error\n); If something fails it's nice to know the error code - you need to print it. For that you need to more GetLastError outside of ok() function call. See other tests as example. +static BOOL get_raw_input_data_wnd_proc_was_called = FALSE; Static variables automatically set to 0 at the start. You don't need to do it explicitly. Vitaliy.
Re: [PATCH 2/3] server: Added server-side raw input function implementations (try 4)
On 06/20/2011 02:55 AM, Vincas Miliūnas wrote:
(see the message in the first part of the patch)
+struct raw_input
+{
+struct listentry;
+unsigned short usage_page;
+unsigned short usage;
+BOOL processed;
+BOOL retrieved;
+RAWINPUT raw;
+
+/* common fields */
+unsigned int type;
+unsigned int message;
+unsigned int flags;
+unsigned int extra_info;
+
+/* mouse fields */
+unsigned int point_x;
+unsigned int point_y;
+unsigned int mouse_data;
+unsigned int cursor_x;
+unsigned int cursor_y;
+
+/* keyboard fields */
+unsigned int vk_code;
+unsigned int scan_code;
+};
Mouse and keyboard fields should be a union to save space.
+#define NUM_RAW_DEVICES (sizeof( raw_devices ) / sizeof( struct raw_device ))
Please use sizeof(array)/sizeof(array[0]) construct instead.
+UINT i, size_in_bytes;
In server please use standard C types (unsigned int in this case).
+/* Currently fake handles are provided, however they are only used for
the GetRawInputDeviceInfo function,
+ thus it should not create any undesirable side effects */
+result[i].hDevice = (HANDLE)raw_devices[i].handle;
This doesn't sound right. What this handle can be used for?
+if (!(raw = malloc( sizeof( *raw ) )))
In server please use mem_alloc() instead.
+/* Unregister a raw input device */
+DECL_HANDLER(unregister_raw_input_device)
I don't see handing of case where device is not found. Should you return
error instead?
+LIST_FOR_EACH( registration_entry, current-process-raw_registered )
+{
+struct raw_registration *registration = LIST_ENTRY(
registration_entry, struct raw_registration, entry );
You can use this instead:
LIST_FOR_EACH_ENTRY( registration, current-process-raw_registered,
struct raw_registration, entry )
+/* Prevent unprocessed raw input entries from being queued indefinitely */
+while (current-process-raw_inputs_len = MAX_RAW_INPUT_QUEUE_LENGTH)
+{
+struct raw_input *head = LIST_ENTRY( list_head(
current-process-raw_inputs ), struct raw_input, entry );
+list_remove( head-entry );
+current-process-raw_inputs_len -= 1;
+free( head );
+}
Looks like you need a circular buffer (an array of fixed size with head and
tail pointers) instead of a list here.
Vitaliy.
Re: Disablin xinput2 for now?
On 06/20/2011 02:31 PM, Austin English wrote: On Mon, Jun 20, 2011 at 09:36, Alexandre Julliardjulli...@winehq.org wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: On 06/20/2011 02:55 AM, Marcus Meissner wrote: As this might be a X.org issue, should we packagers turn off xinput2 support for the time being? If that will fix all dinput regressions - absolutely. Or add a registry option to disable xinput2. It will of course fix some regressions, and introduce new ones for things that have been fixed by the changes. Don't tell me that dinput was working perfectly before. And there is a registry option (GrabPointer). I just did a quick test with today's git (wine-1.3.22-255-g4c0c0d3) (which has http://source.winehq.org/git/wine.git/commitdiff/adb86c5f2a2584dc8131b08d26aef7c82910d0b5). Using Duke Nukem Forever, which has the mouse jumping bug (http://bugs.winehq.org/show_bug.cgi?id=27156) plain wine: I can move normally, with occasional mouse jumps. wine + GrabPointer=N: mouse spins around uncontrollably, a la bug http://bugs.winehq.org/show_bug.cgi?id=22282 wine configured with --without-xinput2: mouse movement isn't fluid at all. Instead, the only way the cursor will move is jumps in a random direction (like bug 27156, except there is no normal behavior in between the jumps). Can you check how well it works with wine-1.3.19 - before all xinput2 related changes went in? Vitaliy.
Re: Disablin xinput2 for now?
On 06/20/2011 08:36 AM, Alexandre Julliard wrote: Vitaliy Margolenwine-de...@kievinfo.com writes: On 06/20/2011 02:55 AM, Marcus Meissner wrote: As this might be a X.org issue, should we packagers turn off xinput2 support for the time being? If that will fix all dinput regressions - absolutely. Or add a registry option to disable xinput2. It will of course fix some regressions, and introduce new ones for things that have been fixed by the changes. Don't tell me that dinput was working perfectly before. And there is a registry option (GrabPointer). Dinput never perfectly worked before and doesn't work perfectly now. Even native has serious issues in Wine. I've lost track of all the changes you did to mouse/keyboard events, ll hooks, pointer position changes, etc. But it seems to me something broke that was working before. At least did not affect programs as much as it does now. I've seen number of programs/games that were highly sensitive to timing and order of various events. Unfortunately it was impossible to exactly replicate and/or make a test for it. And with dynamic nature of the problem it's a royal PITA to debug. In either case, it seems that what used to more-less work before doesn't work at all. Or has major problems (non-xinput2 code-path). And the new xinput2 code-path while resolving large number of issues, introduced number of other problems. So just disabling xinput2 support in any a binary build won't really help much. Vitaliy.
Re: [PATCH 3/6] dinput: SetActionMap setting the device buffer.
On 06/20/2011 04:06 PM, Lucas Fialho Zawacki wrote:
@@ -1391,9 +1391,24 @@ HRESULT WINAPI
IDirectInputDevice8AImpl_SetActionMap(LPDIRECTINPUTDEVICE8A iface
LPCSTR lpszUserName,
DWORD dwFlags)
{
...
+hr = IDirectInputDevice8_SetActionMap(This-IDirectInputDevice8W_iface,
+diafW, NULL, dwFlags);
You need to call IDirectInputDevice8WImpl_SetActionMap directly here. Also
please don't wrap lines at 80 characters, you can put all that into one
line. And please be consistent with spacing around parentheses - either put
spaces on both sides, or don't put at all.
+memset(dp,0,sizeof(dp));
Please don't needlessly zero structures. You setting most values of it
anyway. And please don't forget spaces after comas.
Otherwise looks really good!
Vitaliy.
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 06/20/2011 09:36 PM, Austin Lund wrote: On 20 June 2011 23:52, Vitaliy Margolenwine-de...@kievinfo.com wrote: +SERVER_START_REQ( create_snapshot ) You not checking if this call succeeded or not. I've tried to include all the checks in the new patch. +while (sizeof(ULONG) + sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count= len) You don't need a loop to calculate new size. I've done this a different way in the new patch which avoids the reallocs. You leaking old shi here. Also why do you need to zero allocated memory if you assigning all of it valid values? Not all of the values are assigned. Only the handle pid is accessible with the current wineserver protocol. The remainder of the fields are best set to zero as this is a sane value for these fields to trigger exceptions and figure out how/when these values are being accessed (e.g. null pointers or null handles). Later patches, which I have already sent, try to fill in more of these fields. Makes sense. The patch looks good to me. Vitaliy.
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 06/19/2011 08:15 PM, Austin Lund wrote:
+ret = wine_server_call( req );
+shi-Count += reply-handles;
+len = sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count +
sizeof(ULONG);
+shi = RtlReAllocateHeap(GetProcessHeap(),
HEAP_ZERO_MEMORY, shi, len);
Please do allocations outside of server call block. And add handling of
failed realloc. Also please double the allocated size, don't reallocate
after each server call.
+for (i = shi-Count - reply-handles; i shi-Count; i++)
{
+shi-Handle[i].OwnerPid = reply-pid;
+}
Please follow file's curly braces style - none for single line blocks, or on
separate line for multi-line blocks.
Vitaliy.
