March 2013 - Wine-Devel - list.winehq.org

Problem about declarations of strcoll functions

by Jactry Zeng

Hi folks, I tried to implement a lot of strcoll function these days, and I meet some trouble when I added the declaration into include/msvcrt/*.h. With "grep coll *.spec" in dlls/msvcrt/ we can find there was a lot of strcoll function. Some of them was implemented and some of them not: @ cdecl ___lc_collate_cp_func() @ stub __lc_collate @ extern __lc_collate_cp MSVCRT___lc_collate_cp @ cdecl _mbscoll(str str) @ cdecl _mbscoll_l(str str ptr) @ cdecl _mbsicoll(str str) @ cdecl _mbsicoll_l(str str ptr) @ cdecl _mbsnbcoll(str str long) @ cdecl _mbsnbcoll_l(str str long ptr) @ cdecl _mbsnbicoll(str str long) @ cdecl _mbsnbicoll_l(str str long ptr) @ stub _mbsncoll(str str long) # stub _mbsncoll_l(str str long ptr) @ stub _mbsnicoll(str str long) # stub _mbsnicoll_l(str str long ptr) @ cdecl _strcoll_l(str str ptr) MSVCRT_strcoll_l @ cdecl _stricoll(str str) MSVCRT__stricoll @ cdecl _stricoll_l(str str ptr) MSVCRT__stricoll_l @ cdecl _strncoll(str str long) MSVCRT_strncoll_l @ cdecl _strncoll_l(str str long ptr) MSVCRT_strncoll @ cdecl _strnicoll(str str long) MSVCRT__strnicoll @ cdecl _strnicoll_l(str str long ptr) MSVCRT__strnicoll_l # stub _wcscoll_l(wstr wstr ptr) @ cdecl _wcsicoll(wstr wstr) MSVCRT__wcsicoll # stub _wcsicoll_l(wstr wstr ptr) @ cdecl _wcsncoll(wstr wstr long) MSVCRT__wcsncoll @ cdecl _wcsncoll_l(wstr wstr long ptr) MSVCRT__wcsncoll_l @ cdecl _wcsnicoll(wstr wstr long) MSVCRT__wcsnicoll # stub _wcsnicoll_l(wstr wstr long ptr) @ cdecl strcoll(str str) MSVCRT_strcoll @ cdecl wcscoll(wstr wstr) MSVCRT_wcscoll With "grep coll *.h" in include/msvcrt/ we can find some declarations of some strcoll function: crtdefs.h: unsigned int lc_collate_cp; mbstring.h:int __cdecl _mbscoll(const unsigned char*,const unsigned char*); mbstring.h:int __cdecl _mbsicoll(const unsigned char*,const unsigned char*); mbstring.h:int __cdecl _mbsnbcoll(const unsigned char*,const unsigned char*,size_t); mbstring.h:int __cdecl _mbsnbicoll(const unsigned char*,const unsigned char*,size_t); mbstring.h:int __cdecl _mbsncoll(const unsigned char*,const unsigned char*,size_t); mbstring.h:int __cdecl _mbsnicoll(const unsigned char*,const unsigned char*,size_t); string.h:int __cdecl _stricoll(const char*,const char*); string.h:int __cdecl strcoll(const char*,const char*); string.h:int __cdecl _wcsicoll(const wchar_t*,const wchar_t*); string.h:int __cdecl _wcsnicoll(const wchar_t*,const wchar_t*,size_t); string.h:int __cdecl wcscoll(const wchar_t*,const wchar_t*); string.h:static inline int stricoll(const char* s1, const char* s2) { return _stricoll(s1, s2); } string.h:static inline int wcsicoll(const wchar_t* str1, const wchar_t* str2) { return _wcsicoll(str1, str2); } wchar.h:int __cdecl _wcsicoll(const wchar_t*,const wchar_t*); wchar.h:int __cdecl _wcsnicoll(const wchar_t*,const wchar_t*,size_t); wchar.h:int __cdecl wcscoll(const wchar_t*,const wchar_t*); As we can see, _strcoll_l and _mbsnbcoll_l were implemented but they were not declared. Should we declare all of them? And whether the sequence was important? Thank you. -- Regards, Jactry Zeng

March 31, 2013

Re: Float printf precision

by Henri Verbeet

On 31 March 2013 11:58, Stefan Dösinger <stefandoesinger(a)gmail.com> wrote: > I was curious how you ended up with 8 digits. My thinking was that 32 > bit floats have a 23 bit mantissa, and one digit in the printout > encodes lb(10) bits - so you need ~6,9 digits to encode the mantissa, > which rounds up to 7. Thinking further, %.6e should do the job as > well, as there's one additional digit leading the decimal. > > Is my thinking correct, or am I missing something? > It's been a while since I actually did the calculation, but IIRC you also need to take the implicit extra bit in the significand into account, so you'd end up with log(2^24) = ~7.2, which would give %.7e. But I just rounded up to the next "round" number.

March 31, 2013

Float printf precision

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Henri, While overhauling the patch to hardcode local constants in the shader code I came across the TRACEs in wined3d_device_set_{vs,ps}_consts_f, which print the floats with %.8e. You changed this from %f in 979e3744f7721bef9e88fd671dfd7591c9f85392. I was curious how you ended up with 8 digits. My thinking was that 32 bit floats have a 23 bit mantissa, and one digit in the printout encodes lb(10) bits - so you need ~6,9 digits to encode the mantissa, which rounds up to 7. Thinking further, %.6e should do the job as well, as there's one additional digit leading the decimal. Is my thinking correct, or am I missing something? Cheers, Stefan -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRWAiyAAoJEN0/YqbEcdMw+4AQAIW57gxqJJcPEKzS35K4R++Q aMy1rma7JV4/cUmRIqkmOROc6Y3T2bz7x3e12mNpLVvI9YUBdaZKsofuBJk9QCtL R6nWL266dLc3aRdIFazwjxK6GmohqjxoLidpCAPSXGkekjw48DqhA+5fdXO2YYm/ 2g1Fhyy/cpi6BN2AUh760YNy3ojk/x9eH25oTmifFxCUbWlhYNGsIVt8OTTW9YBE RnQyyoO2lPjNi5mlNIqoSkkYriG5m97nS8bSMXvu9BjKW8rSmmErLGPf2zDL7Big wdibKxbXaSSx74upJaOTLjoOx04sqoi4SuPnX/55ftFKj7s2SeoFJaOUwgdNJoN+ UxKcd0HzfaXBi/VikYvtfMd0+SLA8wFIVjrJsWeBwgNU9cEAA3OiRFPWyizZCPqu 4UQOSnxuK6LZ7Ke58ikcG6JqMVTye8iTeEmr6r3NhkRdmCKrFbVGmJ1POb8Q26Hu q2yQBHq2L5CZ0Vx7fruvqq9Ap8auI/l5B6tneDM/RUdvpWxQTJneVRpSqIE50ZmW U3XH2cx4eLaEGJnSnHBgxnqgSAmcVTJEkFHtd8tCiqsPcnkw2g3Ng3gQZhjkDJ45 ZxvXNxVUl3B899Y1CwFd3FVJhKQq2jFS0NQvNQYZXhx0djZ2ok8rvtmoJzTaNN1s SMfTtZDHxHFtluASd6zJ =7CAL -----END PGP SIGNATURE-----

March 31, 2013

Re: wined3d: Properly handle WS_VISIBLE and WS_EX_TOPMOST style flags on fullscreen windows.

by Henri Verbeet

On 30 March 2013 23:50, Sam Edwards <cfsworks(a)gmail.com> wrote: > I'm a bit confused, then: Without this change, the exstyle test breaks, and the guidelines request that every patch be atomic. They also request that all tests be included in the same patch. I'm not sure there really is such a rule. > Off the top of my head, I would guess it's one of these: > 1. The "all tests in the same patch" rule is only appropriate for single-patch submissions, so in multipatch patchsets, it's acceptable to include a final patch with nothing but tests. > 2. Only introduce the tests in the last patch of the patchset. (That is, add them all at once, but only when all of the necessary changes have been made to allow the tests to work first.) > 3. Include each test in the first patch that will allow the test to pass. (That is, add them incrementally.) > 4. Add the tests as todo_wine in the first patch, then remove the todo_wine in later patches as each test becomes functional. (That is, add them all at once as todo_wine, and mark them as non-todo incrementally.) I think either or those is fine in principle, it depends a bit on the kind of test what's most practical. I guess that for this one you'll probably want 3.

March 31, 2013

[website try2] Simplified Chinese translation for release 1.5.27

by Jactry Zeng

March 31, 2013

Re: wined3d: Properly handle WS_VISIBLE and WS_EX_TOPMOST style flags on fullscreen windows.

by Sam Edwards

On 03/29/2013 06:16 AM, Henri Verbeet wrote: > I think the idea is basically ok, but I do have some comments: > > I think test_window_style() is a more appropriate place for the tests, > and you should add them to the variants in ddraw as well. Ddraw > applications in particular are very fragile in this regard. Note that > the test_window_style() tests are fairly similar to what your tests > do, except that they don't test restoring the window as such. That > test also shows that we're actually not supposed to touch the styles > at all, but that's hard to make work without calling into winex11. Ack, you're right! I saw the test_window_style() but second-guessed myself and put it into test_reset() instead... That was foolish of me. Thanks. >> - SetWindowPos(window, HWND_TOP, 0, 0, w, h, SWP_FRAMECHANGED | SWP_SHOWWINDOW | SWP_NOACTIVATE); >> + SetWindowPos(window, HWND_TOPMOST, 0, 0, w, h, SWP_FRAMECHANGED | SWP_SHOWWINDOW | SWP_NOACTIVATE); > This is really a separate change. I'm a bit confused, then: Without this change, the exstyle test breaks, and the guidelines request that every patch be atomic. They also request that all tests be included in the same patch.What is the proper procedure for this when it comes to multipatch patchsets? Off the top of my head, I would guess it's one of these: 1. The "all tests in the same patch" rule is only appropriate for single-patch submissions, so in multipatch patchsets, it's acceptable to include a final patch with nothing but tests. 2. Only introduce the tests in the last patch of the patchset. (That is, add them all at once, but only when all of the necessary changes have been made to allow the tests to work first.) 3. Include each test in the first patch that will allow the test to pass. (That is, add them incrementally.) 4. Add the tests as todo_wine in the first patch, then remove the todo_wine in later patches as each test becomes functional. (That is, add them all at once as todo_wine, and mark them as non-todo incrementally.) >> + device->style ^= (device->style^style) & WS_VISIBLE; > Spaces around the (second) ^, please. Done. :) Thank you, Sam

March 30, 2013

Fresh WineWiki Theme, Straight from the Farm

by Kyle Auble

So as usual, my estimate of how long it would take was way off, but I'm finally done adapting the WineHQ theme to Moinmoin. It's not perfect, but I feel it's ready to submit for switching over. I've tested it in Firefox, Chromium, Midori, and Opera (briefly in IE8 & IE9 too). I did do everything through Moinmoin's stand-alone server, but while I know there always seem to be ghosts in the machine, I'm guessing that Apache shouldn't interfere with layout issues. I mostly followed the WineHQ & current WineWiki theme for large things, just tweaking the CSS to make it fit what Moinmoin spits out. I did make more design choices of my own on the smaller stuff though. After wrestling with Moinmoin's theme system, I was surprised how many things are still hard-coded (at least as of v1.5). For example, Moinmoin wraps some things in divs without providing a CSS class for the div, just the child element. The fortune cookie on the RecentChanges page & the edit page are two cases of that. The dummy content in the search bar is another tricky one. It's set in the javascript for the search bar (don't remember where it was in the source tree) so I left that one alone. One other tricky issue was handling overflows & RTL support. They both seem to work ok in isolation, but go crazy when you mix them. I haven't checked RTL in IE, but if you force an RTL page to overflow in Webkit, it positions things based on what's actually in the window (so the sidebar winds up floating in the middle of the page). Firefox simply doesn't try to apply any of the overflow rules in RTL (which I guess is nice in a Hippocratic sense). Opera alone handled things exactly like I hoped. As for the tables, I think they should display ok now, although I don't know if everyone wants my exact values for paddings & such. The old theme had some table-based layout that may have had conflicting CSS rules, but I've div-vied everything up so the only table tags now should be when Moinmoin itself chooses them over a div and well... tables. I've pushed all of the changes to my Bitbucket repo at: https://bitbucket.org/kauble/wine-wiki-migration The "refined" branch is the one you want although I expect that once the code is hosted at WineHQ's git repo, it would become the new master branch. At that point, I can just reset my repo to point to the WineHQ one as the origin. - Kyle

March 30, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Juan Lang

Hi Jacek, On Sat, Mar 30, 2013 at 8:36 AM, Jacek Caban <jacek(a)codeweavers.com> wrote: > Most of the argument could be used against enabling TLS 1.1 and TLS 1.2, > because it's not present on older Macs (nor enabled by default on Windows), > so we'll have different behaviour. That's sadly something we have to live > with. Anyway, I'm all for trying to avoid using SSL2, but I'd like to be a > bit less extreme. How about this patch: > > http://source.winehq.org/patches/data/95298 > > With this patch, SSL2 will be completely disabled in default Wine config, > but it will be still possible to enable by registries, if someone really > needs it. > Thanks, this approach looks good to me. --Juan

March 30, 2013

Re: Windows API on Linux

by Vincent Povirk

Mono on non-Windows platforms ships with a static library called io-layer that implements a small subset of the Windows API for compatibility reasons, without any server processes or a registry. It does not (normally) attempt to do any path translation, so I think that's as close as possible to what you want to achieve for those low-level api's. Of course, there's a limit to the level of compatibility (even if you're only looking for source compatibility) you can achieve without the sort of architecture Wine has to simulate services that are available on a real Windows system. Combine this with the fact that the Windows API just isn't very nice to program against, and it leads to little interest in a project like this. But maybe it would be possible for one person to pull this off for a large enough win32 subset to run a GUI application, using the code already available in Mono and Wine.

March 30, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Jacek Caban

Hi Juan, On 03/29/13 18:19, Juan Lang wrote: > Hi Jacek, > > thanks for the detailed reply. > > On Fri, Mar 29, 2013 at 3:02 AM, Jacek Caban <jacek(a)codeweavers.com > <mailto:jacek(a)codeweavers.com>> wrote: > > Each protocol has two kinds of enable/disable flags: "enabled" and > "disabled by default". Those have different default values for > each protocol and there are registry setting allowing to change > each of them. Only "enabled" protocols are used at all. This patch > limits "enabled" protocols to those that we can really support. If > an application asks schannel to use default set of protocols > (which I'd expect them to do unless they have a good reason), > schannel will use all "enabled" protocols that are not "disabled > by default". An alternative to default set of protocols is listing > each allowed separately. > > This means that if protocol is "enabled" and "disabled by default" > it won't be used unless application explicitly asks for it. SSL2 > is such a protocol by default. Do you think we should do this > differently? > > > Yes, my suggestion here is to explicitly disable SSL2 support > altogether. GnuTLS doesn't support it, and having behavior that > differs between Linux and Mac can be kind of maddening. I can imagine > something like, "embedded browser doesn't work for game X", with lots > of "works for me" reports and the occasional "fails here too", only to > discover that it works on Mac but not Linux. The additional cost of a > difference in behavior doesn't seem worth it, especially when the > protocol itself really should have died long ago. Most of the argument could be used against enabling TLS 1.1 and TLS 1.2, because it's not present on older Macs (nor enabled by default on Windows), so we'll have different behaviour. That's sadly something we have to live with. Anyway, I'm all for trying to avoid using SSL2, but I'd like to be a bit less extreme. How about this patch: http://source.winehq.org/patches/data/95298 With this patch, SSL2 will be completely disabled in default Wine config, but it will be still possible to enable by registries, if someone really needs it. Jacek

March 30, 2013

Re: [website] Simplified Chinese translation for release 1.5.27

by Hin-Tak Leung

--- On Sat, 30/3/13, Jactry Zeng <jactry92(a)gmail.com> wrote: From: Jactry Zeng <jactry92(a)gmail.com> Subject: Re: [website] Simplified Chinese translation for release 1.5.27 To: htl10(a)users.sourceforge.net Cc: "Wine Devel" <wine-devel(a)winehq.org>, "Qian Hong" <fracting(a)gmail.com> Date: Saturday, 30 March, 2013, 10:41 2013/3/30 Hin-Tak Leung <htl10(a)users.sourceforge.net> > > > I think both are terrible translations but could not think of a better alternative off my head, so I went to consult google translate. It defaults to 運行時庫 for "runtime library" but allow me to choose a few other alternatives, among which is > > "執行階段程式庫" > > Don't you think that's much better!? Hi Hin-Tak, Thanks for your review. I also think both "运行时" or "运行时库" was not best translation. But I have seen a lot of documents in Simplified Chinese translated it in this way. Maybe it was not too accurate... And in my opinion, "运行时" meaning some function, "运行时库" meaning a runtime library. To be honest, I think a lot of Chinese translations of technical topics (or even not-technical topics) are quite poor, and little beyond a word-for-word substitution. run -> 运行 ("to convey/process", the more common use of "run", of 'using your legs faster than walking' is obviously wrong) time -> 时 ("hour", "during") library -> 库 ("storage container", the more common use of "library" to mean "collection of books" obviously wrong). Whereas "執行階段程式庫" meaning "the program/routine library of/in the execute stage" is quite a lot more fitting to what 'runtime library' actually means. As I was saying, one needs to be vigilant about automated translations being 'not quite correct'. I suspect it is a much abused practice in China, so you see the same (mis-)usage over and over. There are entire web sites devoted to poorly translated public signs in China and making fun of them. It is quite educational - one gets lazy and and accepts the occasional poor translation, but seeing similar but slightly different mistakes collected in a large quantity really makes one think. 2013/3/30 Jactry Zeng <jactry92(a)gmail.com> > > I translated it as "高质量" meaning a picture have hight resolution. Here has a typo, hight -> higher .... -- Regards, Jactry Zeng

March 30, 2013

Re: [website] Simplified Chinese translation for release 1.5.27

by Hin-Tak Leung

This one I do without google translate, "high resolution" would be 高解像 . --- On Sat, 30/3/13, Jactry Zeng <jactry92(a)gmail.com> wrote: From: Jactry Zeng <jactry92(a)gmail.com> Subject: Re: [website] Simplified Chinese translation for release 1.5.27 To: "Qian Hong" <fracting(a)gmail.com> Cc: "Wine Devel" <wine-devel(a)winehq.org> Date: Saturday, 30 March, 2013, 10:23 2013/3/30 Qian Hong <fracting(a)gmail.com> > 2013/3/30 Jactry Zeng <jactry92(a)gmail.com>: > > + <li>一些 C 运行时的改进。</li> > > I think it is better to translate "C runtime" to “C 运行时库“ rather than > just ”C 运行时“。 > > + <li>为内建程序提供更高质量的图标。</li> > > Large icon means 大图标，not 高质量图标。You can verify this by the git commits。In fact, I have verified it before I commit this patch. I translated it as "高质量" meaning a picture have hight resolution. For a example, programs/wordpad/wordpad.ico, had a 48x48 resolution before, and now it is 256x256. > Thanks for the work. Thanks for your review. I will commit a newer patch. :-) -- Regards, Jactry Zeng -----Inline Attachment Follows-----

March 30, 2013

Re: Windows API on Linux

by Joris Huizer

On 03/29/2013 01:27 PM, Abhinav Jangda wrote: > Hello everyone, I'm working on a project which aims to develop a port > of Windows API on Linux, so that programs written using Windows API > could directly be compiled and executed on Linux with its native look > and feel. > Anyone, interested in working on this project. I think some Wine > developers will be as they would be having a good knowledge of Windows > API. Reminds me of Winelib Joris

March 30, 2013

Re: [website] Simplified Chinese translation for release 1.5.27

by Jactry Zeng

2013/3/30 Qian Hong <fracting(a)gmail.com> > 2013/3/30 Jactry Zeng <jactry92(a)gmail.com>: > > + <li>一些 C 运行时的改进。</li> > > I think it is better to translate "C runtime" to “C 运行时库“ rather than > just ”C 运行时“。 > > + <li>为内建程序提供更高质量的图标。</li> > > Large icon means 大图标，not 高质量图标。You can verify this by the git commits。 In fact, I have verified it before I commit this patch. I translated it as "高质量" meaning a picture have hight resolution. For a example, programs/wordpad/wordpad.ico, had a 48x48 resolution before, and now it is 256x256. > Thanks for the work. Thanks for your review. I will commit a newer patch. :-) -- Regards, Jactry Zeng

March 30, 2013

Re: [website] Simplified Chinese translation for release 1.5.27

by Hin-Tak Leung

I was not very happy with "大型 ..." for "large icon", so I expanded the meaning a bit and did 'large size icon'. I think it should be 大規格 or 大尺寸 or 大碼 . All of them are more precise about "size", whereas "大型" is less precise and more generic as in 'large format'/'large style'. So I would opt for "大碼圖標" . --- On Sat, 30/3/13, Hin-Tak Leung <htl10(a)users.sourceforge.net> wrote: > From: Hin-Tak Leung <htl10(a)users.sourceforge.net> > Subject: Re: [website] Simplified Chinese translation for release 1.5.27 > To: "Wine Devel" <wine-devel(a)winehq.org>, fracting(a)gmail.com, "Jactry Zeng" <jactry92(a)gmail.com> > Date: Saturday, 30 March, 2013, 10:02 > --- On Sat, 30/3/13, Qian Hong <fracting(a)gmail.com> > wrote: > > > Hi Jactry, > > > > 2013/3/30 Jactry Zeng <jactry92(a)gmail.com>: > > > + <li>一些 C > 运行时的改进。</li> > > > > I think it is better to translate "C runtime" to “C > > 运行时库“ rather than > > just ”C 运行时“。 > > I think both are terrible translations but could not think > of a better alternative off my head, so I went to consult > google translate. It defaults to 運行時庫 for "runtime > library" but allow me to choose a few other alternatives, > among which is > > "執行階段程式庫" > > Don't you think that's much better!? > > I think google translate is quite useful for technical terms > like these, as long as one is vigilant about not accepting > 'not quite correct' and go through the alternatives. One can > also help it a bit by giving a more verbose version of the > concept ('runtime library' instead of just 'runtime'), and > even occasionally some context (e.g. a full > sentence with '...loading in a runtime library...'). > > > + > > > <li>为内建程序提供更高质量的图标。</li> > > > > Large icon means 大图标，not 高质量图标。You > can > > verify this by the git commits。 > > 大型圖示 or 大型圖標 ? > > > Thanks for the work. > > > > -- > > Regards, > > Qian Hong > > > > - > > http://www.winehq.org > >

March 30, 2013

Re: [website] Simplified Chinese translation for release 1.5.27

by Hin-Tak Leung

--- On Sat, 30/3/13, Qian Hong <fracting(a)gmail.com> wrote: > Hi Jactry, > > 2013/3/30 Jactry Zeng <jactry92(a)gmail.com>: > > + <li>一些 C 运行时的改进。</li> > > I think it is better to translate "C runtime" to “C > 运行时库“ rather than > just ”C 运行时“。 I think both are terrible translations but could not think of a better alternative off my head, so I went to consult google translate. It defaults to 運行時庫 for "runtime library" but allow me to choose a few other alternatives, among which is "執行階段程式庫" Don't you think that's much better!? I think google translate is quite useful for technical terms like these, as long as one is vigilant about not accepting 'not quite correct' and go through the alternatives. One can also help it a bit by giving a more verbose version of the concept ('runtime library' instead of just 'runtime'), and even occasionally some context (e.g. a full sentence with '...loading in a runtime library...'). > + > <li>为内建程序提供更高质量的图标。</li> > > Large icon means 大图标，not 高质量图标。You can > verify this by the git commits。大型圖示 or 大型圖標 ? > Thanks for the work. > > -- > Regards, > Qian Hong > > - > http://www.winehq.org

March 30, 2013

Re: [website] Simplified Chinese translation for release 1.5.27

by Qian Hong

Hi Jactry, 2013/3/30 Jactry Zeng <jactry92(a)gmail.com>: > + <li>一些 C 运行时的改进。</li> I think it is better to translate "C runtime" to “C 运行时库“ rather than just ”C 运行时“。 + <li>为内建程序提供更高质量的图标。</li> Large icon means 大图标，not 高质量图标。You can verify this by the git commits。 Thanks for the work. -- Regards, Qian Hong - http://www.winehq.org

March 30, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Juan Lang

Hi Jacek, thanks for the detailed reply. On Fri, Mar 29, 2013 at 3:02 AM, Jacek Caban <jacek(a)codeweavers.com> wrote: > Each protocol has two kinds of enable/disable flags: "enabled" and > "disabled by default". Those have different default values for each > protocol and there are registry setting allowing to change each of them. > Only "enabled" protocols are used at all. This patch limits "enabled" > protocols to those that we can really support. If an application asks > schannel to use default set of protocols (which I'd expect them to do > unless they have a good reason), schannel will use all "enabled" protocols > that are not "disabled by default". An alternative to default set of > protocols is listing each allowed separately. > > This means that if protocol is "enabled" and "disabled by default" it > won't be used unless application explicitly asks for it. SSL2 is such a > protocol by default. Do you think we should do this differently? > Yes, my suggestion here is to explicitly disable SSL2 support altogether. GnuTLS doesn't support it, and having behavior that differs between Linux and Mac can be kind of maddening. I can imagine something like, "embedded browser doesn't work for game X", with lots of "works for me" reports and the occasional "fails here too", only to discover that it works on Mac but not Linux. The additional cost of a difference in behavior doesn't seem worth it, especially when the protocol itself really should have died long ago. --Juan

March 29, 2013

Re: [PATCH 1/3] jscript: Use helpers to access string buffers in jsregexp.c.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > --- > dlls/jscript/jsregexp.c | 114 > +++++++++++++++++++++++++++++++----------------- > 1 file changed, 73 insertions(+), 41 deletions(-) It breaks the tests: ../../../tools/runtest -q -P wine -M jscript.dll -T ../../.. -p jscript_test.exe.so run.c && touch run.ok run.c:1804: Test failed: expected global_success_d run.c:1805: Test failed: expected global_success_i run.c:1807: Test failed: parse_script failed: 80004005 run.c:1804: Test failed: expected global_success_d run.c:1805: Test failed: expected global_success_i run.c:1807: Test failed: parse_script failed: 80004005 run.c:1804: Test failed: expected global_success_d run.c:1805: Test failed: expected global_success_i run.c:1807: Test failed: parse_script failed: 80004005 run.c:1804: Test failed: expected global_success_d run.c:1805: Test failed: expected global_success_i run.c:1807: Test failed: parse_script failed: 80004005 run.c:1804: Test failed: expected global_success_d run.c:1805: Test failed: expected global_success_i run.c:1807: Test failed: parse_script failed: 80004005 run.c:1804: Test failed: expected global_success_d run.c:1805: Test failed: expected global_success_i run.c:1807: Test failed: parse_script failed: 80004005 make: *** [run.ok] Error 18 -- Alexandre Julliard julliard(a)winehq.org

March 29, 2013

Re: d3dx9 [patch 1/2, try 4]: Implement D3DXSHEvalSphericalLight

by Alexandre Julliard

Nozomi Kodama <nozomi.kodama(a)yahoo.com> writes: > From bb40ba80a24a954d21c44cbe55b4b9c79e2087c5 Mon Sep 17 00:00:00 2001 > From: "nozomi.kodama" <nozomi.kodama(a)yahoo.com> > Date: Wed, 27 Mar 2013 22:38:51 -1000 > Subject: d3dx9: Implement D3DXSHEvalSphericalLight It still fails the same way... -- Alexandre Julliard julliard(a)winehq.org

March 29, 2013

Re: msvcp110: Add VS2012 C/C++ runtime

by Piotr Caban

On 03/28/13 23:22, Stefan Leichter wrote: > --- > configure.ac | 1 + > dlls/msvcp110/Makefile.in | 5 + > dlls/msvcp110/msvcp110.c | 37 + > dlls/msvcp110/msvcp110.spec | 3900 > +++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 3943 insertions(+), 0 deletions(-) > create mode 100644 dlls/msvcp110/Makefile.in > create mode 100644 dlls/msvcp110/msvcp110.c > create mode 100644 dlls/msvcp110/msvcp110.spec You can use tools/make_specfiles script to generate forwards to other msvcpXX dlls. The script will generate big parts of the spec file differently (because it prefers to forward to msvcp90, not to msvcp100).

March 29, 2013

Re: Windows API on Linux

by Henri Verbeet

On 29 March 2013 13:27, Abhinav Jangda <abhijangda(a)hotmail.com> wrote: > Hello everyone, I'm working on a project which aims to develop a port of > Windows API on Linux, so that programs written using Windows API could > directly be compiled and executed on Linux with its native look and feel. Is that really any different from what Wine does?

March 29, 2013

Windows API on Linux

by Abhinav Jangda

Hello everyone, I'm working on a project which aims to develop a port of Windows API on Linux, so that programs written using Windows API could directly be compiled and executed on Linux with its native look and feel. Anyone, interested in working on this project. I think some Wine developers will be as they would be having a good knowledge of Windows API. If anyone interested then please let me know. Thank You :)

March 29, 2013

Re: wined3d: Properly handle WS_VISIBLE and WS_EX_TOPMOST style flags on fullscreen windows.

by Henri Verbeet

On 29 March 2013 03:31, Sam Edwards <cfsworks(a)gmail.com> wrote: > Note that the tests are NOT (primarily) intended to make sure that > WS_VISIBLE and WS_EX_TOPMOST are still set. The main purpose of this patch > is to fix a slightly more serious bug: > > - Create a window without WS_VISIBLE. > - Create a fullscreen Direct3D device. > - wined3d applies its own flags (including WS_VISIBLE) to the window, to > make it fullscreen. > - Reset the device back to windowed mode. > - Because wined3d_device_setup_fullscreen_window didn't see the window had > WS_VISIBLE, wined3d_device_restore_fullscreen_window won't restore the > style. > > This patch corrects that behavior, and the window restores back to its > original style as on Windows. > I think the idea is basically ok, but I do have some comments: I think test_window_style() is a more appropriate place for the tests, and you should add them to the variants in ddraw as well. Ddraw applications in particular are very fragile in this regard. Note that the test_window_style() tests are fairly similar to what your tests do, except that they don't test restoring the window as such. That test also shows that we're actually not supposed to touch the styles at all, but that's hard to make work without calling into winex11. > - SetWindowPos(window, HWND_TOP, 0, 0, w, h, SWP_FRAMECHANGED | SWP_SHOWWINDOW | SWP_NOACTIVATE); > + SetWindowPos(window, HWND_TOPMOST, 0, 0, w, h, SWP_FRAMECHANGED | SWP_SHOWWINDOW | SWP_NOACTIVATE); This is really a separate change. > + device->style ^= (device->style^style) & WS_VISIBLE; Spaces around the (second) ^, please.

March 29, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Jacek Caban

Hi Juan, On 03/28/13 21:55, Juan Lang wrote: > On Thu, Mar 28, 2013 at 12:31 PM, Ken Thomases <ken(a)codeweavers.com > <mailto:ken(a)codeweavers.com>> wrote: > > On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote: > > > --- a/dlls/secur32/schannel_macosx.c > > +++ b/dlls/secur32/schannel_macosx.c > > @@ -630,6 +630,11 @@ static OSStatus > schan_push_adapter(SSLConnectionRef transport, const void *buff, > > return ret; > > } > > > > +DWORD schan_imp_enabled_protocols(void) > > +{ > > + /* NOTE: No support for TLS 1.1 and TLS 1.2 */ > > + return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT | > SP_PROT_TLS1_0_CLIENT; > > > Do we really want to continue supporting SSL2? It's got a number of > vulnerabilities, and is disabled pretty much everywhere by now: > http://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_2.0 I implemented it the way it's done in Windows. It's a bit under-documented and contains usual MSDN mistakes, so let me explain what I found when testing Windows (most of it is implemented by http://source.winehq.org/git/wine.git/commitdiff/0f2e0365ea1f5c6baba4cfd9c0…) Each protocol has two kinds of enable/disable flags: "enabled" and "disabled by default". Those have different default values for each protocol and there are registry setting allowing to change each of them. Only "enabled" protocols are used at all. This patch limits "enabled" protocols to those that we can really support. If an application asks schannel to use default set of protocols (which I'd expect them to do unless they have a good reason), schannel will use all "enabled" protocols that are not "disabled by default". An alternative to default set of protocols is listing each allowed separately. This means that if protocol is "enabled" and "disabled by default" it won't be used unless application explicitly asks for it. SSL2 is such a protocol by default. Do you think we should do this differently? Thanks, Jacek

March 29, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Jacek Caban

Hi Ken, On 03/28/13 20:31, Ken Thomases wrote: > On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote: > >> --- a/dlls/secur32/schannel_macosx.c >> +++ b/dlls/secur32/schannel_macosx.c >> @@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef transport, const void *buff, >> return ret; >> } >> >> +DWORD schan_imp_enabled_protocols(void) >> +{ >> + /* NOTE: No support for TLS 1.1 and TLS 1.2 */ >> + return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT | SP_PROT_TLS1_0_CLIENT; >> +} > Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. You can test at build time with: > > #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080 > ... > #else > ... > #endif > > > If we want to support building on 10.8 for deployment to earlier versions, we'd do something like: > > #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080 > SSLProtocol maxProtocol; > if (SSLGetProtocolVersionMax != NULL && SSLGetProtocolVersionMax(context, &maxProtocol) == noErr) > { > ... compare maxProtocol against kTLSProtocol11 and kTLSProtocol12 ... > } > ... > #else > ... > #endif Thanks for the pointers, I've been meaning to explore it as follow-up. My problem is that I'm still on 10.6 with Xcode 3.2. Would you mind taking care of the patch? > The idea is that SSLGetProtocolVersionMax() would be weak linked, so we'd check if it was actually available before calling it. Of course, the other complication is that that function requires a context parameter, but we can create one just for the query if we're interested in the framework capabilities (as opposed to what's been configured for a particular context). Yes, in this case we're only interested in framework capabilities. We should determine protocols used for given context ourselves, based on caller's requested protocol and confuration, and pass that to framework. Setting up framework is not implemented yet, I have patches for that that I want to test a bit more before sending. Thanks, Jacek

March 29, 2013

Re: [PATCH 1/3] extrac32: Implement more strict command line processing.

by Sergey Guralnik

On 2013-03-26 10:17, Sergey Guralnik wrote: > Is some problem with this series? -- Sergey

March 29, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Juan Lang

On Thu, Mar 28, 2013 at 12:31 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote: > > > --- a/dlls/secur32/schannel_macosx.c > > +++ b/dlls/secur32/schannel_macosx.c > > @@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef > transport, const void *buff, > > return ret; > > } > > > > +DWORD schan_imp_enabled_protocols(void) > > +{ > > + /* NOTE: No support for TLS 1.1 and TLS 1.2 */ > > + return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT | > SP_PROT_TLS1_0_CLIENT; > Do we really want to continue supporting SSL2? It's got a number of vulnerabilities, and is disabled pretty much everywhere by now: http://en.wikipedia.org/wiki/Transport_Layer_Security#SSL_2.0 --Juan

March 28, 2013

Re: secur32: Take schannel backend capabilities into account when configuring enabled protocols.

by Ken Thomases

On Mar 28, 2013, at 6:05 AM, Jacek Caban wrote: > --- a/dlls/secur32/schannel_macosx.c > +++ b/dlls/secur32/schannel_macosx.c > @@ -630,6 +630,11 @@ static OSStatus schan_push_adapter(SSLConnectionRef transport, const void *buff, > return ret; > } > > +DWORD schan_imp_enabled_protocols(void) > +{ > + /* NOTE: No support for TLS 1.1 and TLS 1.2 */ > + return SP_PROT_SSL2_CLIENT | SP_PROT_SSL3_CLIENT | SP_PROT_TLS1_0_CLIENT; > +} Mac OS X 10.8 introduced support for TLS 1.1 and 1.2. You can test at build time with: #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080 ... #else ... #endif If we want to support building on 10.8 for deployment to earlier versions, we'd do something like: #if MAC_OS_X_VERSION_MAX_ALLOWED >= 1080 SSLProtocol maxProtocol; if (SSLGetProtocolVersionMax != NULL && SSLGetProtocolVersionMax(context, &maxProtocol) == noErr) { ... compare maxProtocol against kTLSProtocol11 and kTLSProtocol12 ... } ... #else ... #endif The idea is that SSLGetProtocolVersionMax() would be weak linked, so we'd check if it was actually available before calling it. Of course, the other complication is that that function requires a context parameter, but we can create one just for the query if we're interested in the framework capabilities (as opposed to what's been configured for a particular context). -Ken

March 28, 2013

Re: gdi32: Copy gamma ramp validation from winex11 to make it driver independent (try 2)

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > + /* the bias could be because the app wanted something like a "red shift" > + * like when you're hit in Quake, but XVidMode doesn't support it, > + * so we have to reject a significant bias */ Clearly gdi32 doesn't care about XVidMode. > @@ -1325,6 +1405,18 @@ BOOL WINAPI SetDeviceGammaRamp(HDC hDC, LPVOID ptr) > if (GetObjectType( hDC ) != OBJ_MEMDC) > { > PHYSDEV physdev = GET_DC_PHYSDEV( dc, pSetDeviceGammaRamp ); > + struct gamma_ramp > + { > + WORD red[256]; > + WORD green[256]; > + WORD blue[256]; > + } *ramp = ptr; > + > + if (!check_gamma_ramp(ramp->red) || > + !check_gamma_ramp(ramp->green) || > + !check_gamma_ramp(ramp->blue)) > + return FALSE; This can't work. -- Alexandre Julliard julliard(a)winehq.org

March 28, 2013

Re: winemac: Track drawn surface rect to reduce black flicker for new or resized windows.

by Ken Thomases

On Mar 27, 2013, at 3:31 PM, Ken Thomases wrote: > dlls/winemac.drv/macdrv.h | 12 +++++++++++- > dlls/winemac.drv/surface.c | 16 ++++++++++++++-- > dlls/winemac.drv/window.c | 17 ++--------------- > 3 files changed, 27 insertions(+), 18 deletions(-) > > <0001-winemac-Track-drawn-surface-rect-to-reduce-black-fli.patch> Please don't commit this. I've thought of a different approach. -Ken

March 28, 2013

Re: user32: ImmProcessKey is only called on WM_KEYDOWN and if the message is being removed

by André Hentschel

Am 28.03.2013 14:41, schrieb Aric Stewart: > On 3/28/13 8:10 AM, Dmitry Timoshkov wrote: >> Aric Stewart <aric(a)codeweavers.com> wrote: >> >>>>> Worked out with testing on Japanese windows XP. Resolves issues with >>>>> keystrokes and accelerators in menus when the IME is active. >>>> >>>> You should really start adding the tests and base your patches on test >>>> results instead of some sort of speculation. >>>> >>> I would love to add tests. But things like this require enough parts of >>> things working in such a way that it was very hard if not impossible for >>> me to think of a way to do it through an automated test for wine, let >>> alone one that would run for windows. >> >> So, you admit that your IME patches are just a guesswork, i.e. you have >> zero number of automated tests to check another guess? That's really >> confusing. >> > > No. They are not guesswork. > > I wrote _interactive_ tests that I used on my windows machine to prove the behavior and this patch. It am was unable to make _automated_ tests that will in any meaningful way test the behavior. I just want to mention at this point that there is a interactive mode for winetest: export WINETEST_INTERACTIVE=1 should do it...

March 28, 2013

Re: user32: ImmProcessKey is only called on WM_KEYDOWN and if the message is being removed

by Aric Stewart

On 3/28/13 8:10 AM, Dmitry Timoshkov wrote: > Aric Stewart <aric(a)codeweavers.com> wrote: > >>>> Worked out with testing on Japanese windows XP. Resolves issues with >>>> keystrokes and accelerators in menus when the IME is active. >>> >>> You should really start adding the tests and base your patches on test >>> results instead of some sort of speculation. >>> >> I would love to add tests. But things like this require enough parts of >> things working in such a way that it was very hard if not impossible for >> me to think of a way to do it through an automated test for wine, let >> alone one that would run for windows. > > So, you admit that your IME patches are just a guesswork, i.e. you have > zero number of automated tests to check another guess? That's really > confusing. > No. They are not guesswork. I wrote _interactive_ tests that I used on my windows machine to prove the behavior and this patch. It am was unable to make _automated_ tests that will in any meaningful way test the behavior. -aric

March 28, 2013

Re: user32: ImmProcessKey is only called on WM_KEYDOWN and if the message is being removed

by Dmitry Timoshkov

Aric Stewart <aric(a)codeweavers.com> wrote: > >> Worked out with testing on Japanese windows XP. Resolves issues with > >> keystrokes and accelerators in menus when the IME is active. > > > > You should really start adding the tests and base your patches on test > > results instead of some sort of speculation. > > > I would love to add tests. But things like this require enough parts of > things working in such a way that it was very hard if not impossible for > me to think of a way to do it through an automated test for wine, let > alone one that would run for windows. So, you admit that your IME patches are just a guesswork, i.e. you have zero number of automated tests to check another guess? That's really confusing. -- Dmitry.

March 28, 2013

Re: user32: ImmProcessKey is only called on WM_KEYDOWN and if the message is being removed

by Aric Stewart

On 3/27/13 9:25 PM, Dmitry Timoshkov wrote: > Aric Stewart <aric(a)codeweavers.com> wrote: > >> Worked out with testing on Japanese windows XP. Resolves issues with >> keystrokes and accelerators in menus when the IME is active. > > You should really start adding the tests and base your patches on test > results instead of some sort of speculation. > I would love to add tests. But things like this require enough parts of things working in such a way that it was very hard if not impossible for me to think of a way to do it through an automated test for wine, let alone one that would run for windows. -aric

March 28, 2013

Re: winemac: Null-terminate an snprintf-ed key name buffer in case of short buffer.

by Ken Thomases

On Mar 28, 2013, at 2:26 AM, Per Johansson wrote: > On Wed, Mar 27, 2013 at 7:22 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >> --- > > This has probably been discussed before, but shouldn't snprintfW be > safe against this kind of thing, the way POSIX snprintf is? I actually checked the man page for snprintf() when I originally wrote that code, which is why I assumed snprintfW() would terminate it. So, my vote is for "yes". :) -Ken

March 28, 2013

Re: winemac: Null-terminate an snprintf-ed key name buffer in case of short buffer.

by Per Johansson

On Wed, Mar 27, 2013 at 7:22 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > --- This has probably been discussed before, but shouldn't snprintfW be safe against this kind of thing, the way POSIX snprintf is? Regards, -- Per Johansson

March 28, 2013

Re: user32: ImmProcessKey is only called on WM_KEYDOWN and if the message is being removed

by Dmitry Timoshkov

Aric Stewart <aric(a)codeweavers.com> wrote: > Worked out with testing on Japanese windows XP. Resolves issues with > keystrokes and accelerators in menus when the IME is active. You should really start adding the tests and base your patches on test results instead of some sort of speculation. -- Dmitry.

March 28, 2013

RE: Linker error when improving GetLargestConsoleWindowSize

by Hugh McMaster

-----Original Message----- From: Eric Pouech [mailto:eric.pouech(a)orange.fr] Sent: Wednesday, 27 March 2013 8:54 AM To: wine-devel(a)winehq.org Subject: Re: Linker error when improving GetLargestConsoleWindowSize Ken Thomases wrote: >This won't be able to work. The linker error is telling you, effectively, that you're not importing user32. However, kernel32 can't import user32. Kernel32 is at a lower level than user32. And a console program especially can't rely on user32 being loaded and initialized to satisfy the request you're making. Regards, Ken Eric Pouech wrote: >> actually the correct fix would be to transfer this computation to wineconsole (which already imports user32), but thats' not a really trivial task So, if wineconsole imports user32 to run console programs, that explains why a generic program using SystemParametersInfo or GetSystemMetrics is able to compile and run correctly, yes? For example: --- #include <windows.h> #include <winuser.h> #include <stdio.h> main(void) { RECT workarea; SystemParametersInfo(SPI_GETWORKAREA, 0, &workarea, 0); int screen_x = workarea.right; int screen_y = workarea.bottom; printf("Your screen resolution is %d x %d pixels.\n", screen_x, screen_y); } --- Anyway, if I understand you correctly, the computations would need to be moved to programs/wineconsole (wineconsole.c). Hugh

March 28, 2013

Re: [1/3] kernel32: Set GetLargestConsoleWindowSize based on screen resolution

by Ken Thomases

On Mar 27, 2013, at 6:08 PM, Hugh McMaster wrote: > Ken Thomases wrote: > >>> This approach is untenable. Kernel32 can't be made to depend on X11. It has to still work when X11 isn't available. Also, X11 is just one possible graphics/windowing system that Wine can use, so it's not always the right authority to consult about screen size. > > A fair point. I hadn't considered that windowing systems such as XFree86 were still in use. I'm too used to using distributions running X11. More to the point, Wine doesn't necessarily use any variant of the X Window System. On Mac OS X, it can use the Mac driver. Other drivers are being investigated. >>> Finally, your patch series is broken in the sense that each patch in the series must still be able to compile. You introduce a dependency on screensize.c in the first patch but don't actually provide that source file until the third. > > So patches are applied one by one and then tested? Yes. > I thought patches in a series were applied at the same time. No. A series is used to indicate that later patches can't be applied before the earlier ones. That is, for example, patch 2 requires that patch 1 already was applied before it can be successfully applied. "Successfully applied" means all of: the patch applies, Wine builds, and tests succeed (at least as well as before). Regards, Ken

March 27, 2013

RE: [1/3] kernel32: Set GetLargestConsoleWindowSize based on screen resolution

by Hugh McMaster

-----Original Message----- From: Ken Thomases [mailto:ken(a)codeweavers.com] Sent: Wednesday, 27 March 2013 11:25 PM To: Hugh McMaster Cc: Wine Developers Subject: Re: [1/3] kernel32: Set GetLargestConsoleWindowSize based on screen resolution On Mar 27, 2013, at 6:41 AM, Hugh McMaster wrote: > This patch modifies the dlls/kernel32 Makefile.in to (1) compile a new source file (screensize.c) (patch 3/3) and (2) add -lX11 to the ExtraDllFlags line. Ken Thomases wrote: >> This approach is untenable. Kernel32 can't be made to depend on X11. It has to still work when X11 isn't available. Also, X11 is just one possible graphics/windowing system that Wine can use, so it's not always the right authority to consult about screen size. A fair point. I hadn't considered that windowing systems such as XFree86 were still in use. I'm too used to using distributions running X11. >>Eric Pouech suggested in the other thread to calculate the console window size in wineconsole, at which point you can use user32 and SystemParametersInfo(). (Discussion continued in other thread). >> Finally, your patch series is broken in the sense that each patch in the series must still be able to compile. You introduce a dependency on screensize.c in the first patch but don't actually provide that source file until the third. So patches are applied one by one and then tested? I thought patches in a series were applied at the same time. Hugh

March 27, 2013

Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight

by Alexandre Julliard

Nozomi Kodama <nozomi.kodama(a)yahoo.com> writes: > From 3d331ffa908f69ca58686b4601effbb16d89d1da Mon Sep 17 00:00:00 2001 > From: "nozomi.kodama" <nozomi.kodama(a)yahoo.com> > Date: Thu, 14 Mar 2013 18:34:48 -1000 > Subject: d3dx9: Implement D3DXSHEvalSphericalLight It fails on 64-bit: ../../../../wine/tools/runtest -q -P wine -M d3dx9_36.dll -T ../../.. -p d3dx9_36_test.exe.so ../../../../wine/dlls/d3dx9_36/tests/math.c && touch math.ok math.c:2930: Test failed: Red: case 1, order 5: expected[20] = 0.000529, received 0.000529 math.c:2939: Test failed: Green: case 1, order 5: expected[20] = 0.000809, received 0.000809 math.c:2949: Test failed: Blue: case 1, order 5: expected[20] = 0.001089, received 0.001089 math.c:2930: Test failed: Red: case 1, order 6: expected[20] = 0.000529, received 0.000529 math.c:2939: Test failed: Green: case 1, order 6: expected[20] = 0.000809, received 0.000809 math.c:2949: Test failed: Blue: case 1, order 6: expected[20] = 0.001089, received 0.001089 math.c:2930: Test failed: Red: case 2, order 5: expected[20] = -0.000715, received -0.000714 math.c:2939: Test failed: Green: case 2, order 5: expected[20] = -0.001093, received -0.001093 math.c:2949: Test failed: Blue: case 2, order 5: expected[20] = -0.001471, received -0.001471 math.c:2930: Test failed: Red: case 2, order 6: expected[20] = -0.000715, received -0.000714 math.c:2939: Test failed: Green: case 2, order 6: expected[20] = -0.001093, received -0.001093 math.c:2949: Test failed: Blue: case 2, order 6: expected[20] = -0.001471, received -0.001471 make: *** [math.ok] Error 12 -- Alexandre Julliard julliard(a)winehq.org

March 27, 2013

Re: tools/wine.inf: add HKLM\\System\\CurrentControlSet\\Services\\Eventlog\\Application registry key

by Alexandre Julliard

Austin English <austinenglish(a)gmail.com> writes: > Fixes http://bugs.winehq.org/show_bug.cgi?id=22026 It breaks the tests: ../../../tools/runtest -q -P wine -M advapi32.dll -T ../../.. -p advapi32_test.exe.so eventlog.c && touch eventlog.ok eventlog.c:524: Tests skipped: We don't have a backup eventlog to work with eventlog.c:457: Tests skipped: No records in the 'Application' log eventlog.c:835: Test failed: Expected 1 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 2 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 3 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 4 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 5 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 6 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 7 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 8 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 9 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:835: Test failed: Expected 10 records, got 0 eventlog.c:840: Test failed: Expected oldest to be 1 or higher, got 0 eventlog.c:858: Test failed: Expected 10 records, got 0 eventlog.c:863: Tests skipped: No events were written to the eventlog eventlog.c:1086: Test failed: Expected an eventlog file eventlog.c:1113: Test succeeded inside todo block: Could not delete the registry tree : 0 eventlog.c:1120: Test succeeded inside todo block: Expected MoveFileEx to succeed: 1 make: *** [eventlog.ok] Error 24 -- Alexandre Julliard julliard(a)winehq.org

March 27, 2013

Re: Wiki is down

by Dimi Paun

Thanks for the bug report, it should be fixed now. Dimi. On Wed, Mar 27, 2013 at 12:57 PM, Francois Gouget <fgouget(a)free.fr> wrote: > On Mon, 25 Mar 2013, Dimi Paun wrote: > > > Should be up now, the RAID controller on the box died. > > We'll be moving it to a better box soon, stay tuned. > > Do you know what's up with this message? > > Status of sending notification mails: > [en] fracting: Connection to mailserver 'mail.lattica.com' failed: > (101, 'Network is unreachable') > > Note that it was present before the RAID card trouble so it's not > caused by your recent changes. > > See also bug 33231 - Wiki: Trouble sending notification emails > http://bugs.winehq.org/show_bug.cgi?id=33231 > > -- > Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ > tcA thgirypoC muinelliM latigiD eht detaloiv tsuj evah uoY > -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.

March 27, 2013

Re: Wiki is down

by Francois Gouget

On Mon, 25 Mar 2013, Dimi Paun wrote: > Should be up now, the RAID controller on the box died. > We'll be moving it to a better box soon, stay tuned. Do you know what's up with this message? Status of sending notification mails: [en] fracting: Connection to mailserver 'mail.lattica.com' failed: (101, 'Network is unreachable') Note that it was present before the RAID card trouble so it's not caused by your recent changes. See also bug 33231 - Wiki: Trouble sending notification emails http://bugs.winehq.org/show_bug.cgi?id=33231 -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ tcA thgirypoC muinelliM latigiD eht detaloiv tsuj evah uoY

March 27, 2013

Re: [1/3] kernel32: Set GetLargestConsoleWindowSize based on screen resolution

by Ken Thomases

On Mar 27, 2013, at 6:41 AM, Hugh McMaster wrote: > This patch modifies the dlls/kernel32 Makefile.in to (1) compile a new source file (screensize.c) (patch 3/3) and (2) add -lX11 to the ExtraDllFlags line. This approach is untenable. Kernel32 can't be made to depend on X11. It has to still work when X11 isn't available. Also, X11 is just one possible graphics/windowing system that Wine can use, so it's not always the right authority to consult about screen size. Eric Pouech suggested in the other thread to calculate the console window size in wineconsole, at which point you can use user32 and SystemParametersInfo(). Finally, your patch series is broken in the sense that each patch in the series must still be able to compile. You introduce a dependency on screensize.c in the first patch but don't actually provide that source file until the third. -Ken

March 27, 2013

Re: [PATCH 1/3] msvcr90: Forward some functions directly to ntdll

by Piotr Caban

On 03/27/13 12:13, Alexandre Julliard wrote: > Piotr Caban <piotr(a)codeweavers.com> writes: > >> These functions are currently forwarded from msvcr90 to msvcrt which >> forwards to ntdll. Naver LINE is crashing while trying to call them. > > Does it work if you remove the forward in msvcrt instead? That would be > a better fix. > Yes, I'll send a fixed version.

March 27, 2013

Re: advapi32: Do not terminate a regular program if it calls StartServiceCtrlDispatcher.

by Alexandre Julliard

Alexander Morozov <amorozov(a)etersoft.ru> writes: > @@ -592,7 +586,26 @@ BOOL WINAPI StartServiceCtrlDispatcherA( const SERVICE_TABLE_ENTRYA *servent ) > services[i] = info; > } > > - service_run_main_thread(); > + disp = HeapAlloc( GetProcessHeap(), 0, sizeof(*disp) ); > + > + if (!(disp->manager = OpenSCManagerW( NULL, NULL, SC_MANAGER_CONNECT ))) > + { > + ERR("failed to open service manager error %u\n", GetLastError()); > + HeapFree( GetProcessHeap(), 0, disp ); > + return FALSE; > + } > + > + disp->pipe = service_open_pipe(); > + if (disp->pipe == INVALID_HANDLE_VALUE) > + { > + WARN("failed to create control pipe error %u\n", GetLastError()); > + CloseServiceHandle( disp->manager ); > + HeapFree( GetProcessHeap(), 0, disp ); > + SetLastError( ERROR_FAILED_SERVICE_CONTROLLER_CONNECT ); > + return FALSE; > + } > + > + service_run_main_thread( disp ); There's no reason to duplicate that code, just do it in service_run_main_thread instead. -- Alexandre Julliard julliard(a)winehq.org

March 27, 2013

Re: [PATCH 1/3] msvcr90: Forward some functions directly to ntdll

by Alexandre Julliard

Piotr Caban <piotr(a)codeweavers.com> writes: > These functions are currently forwarded from msvcr90 to msvcrt which > forwards to ntdll. Naver LINE is crashing while trying to call them. Does it work if you remove the forward in msvcrt instead? That would be a better fix. -- Alexandre Julliard julliard(a)winehq.org

March 27, 2013

Re: urlmon: Fix handling of mailto URIs in CoInternetCombineUrlEx.

by Jacek Caban

Hi Hans, On 03/27/13 10:43, Hans Leidekker wrote: > --- > dlls/urlmon/tests/uri.c | 63 +++++++++++++++++++++++++++++++++++++++++++---- > dlls/urlmon/uri.c | 15 ++++++++--- > 2 files changed, 69 insertions(+), 9 deletions(-) > > diff --git a/dlls/urlmon/tests/uri.c b/dlls/urlmon/tests/uri.c > index e5faa81..29bd859 100644 > --- a/dlls/urlmon/tests/uri.c > +++ b/dlls/urlmon/tests/uri.c > @@ -7005,8 +7005,61 @@ static const uri_combine_test uri_combine_tests[] = { > {URL_SCHEME_FILE,S_OK}, > {URLZONE_INVALID,E_NOTIMPL} > } > + }, > + { "http://winehq.org",0, > + "mailto://",0, > + 0,S_OK,FALSE, You are changing parts that are not combining-specific. Since the relative URI is absolute in this case, there is not much happening other than parsing the URI during combining. Those tests are fine, but please add also tests for CreateUri, see uri_tests array. This should be straightforward based on tests you already wrote. > diff --git a/dlls/urlmon/uri.c b/dlls/urlmon/uri.c > index 0f6ee31..c6b5166 100644 > --- a/dlls/urlmon/uri.c > +++ b/dlls/urlmon/uri.c > @@ -422,7 +422,7 @@ static inline BOOL is_hierarchical_uri(const WCHAR **ptr, const parse_data *data > else if(is_hierarchical_scheme(data->scheme_type) && (*ptr)[0] == '\\' && (*ptr)[1] == '\\') { > *ptr += 2; > return TRUE; > - } else if(check_hierarchical(ptr)) > + } else if(data->scheme_type != URL_SCHEME_MAILTO && check_hierarchical(ptr)) > return TRUE; > > *ptr = start; > @@ -2980,6 +2980,9 @@ static BOOL canonicalize_path_opaque(const parse_data *data, Uri *uri, DWORD fla > const BOOL known_scheme = data->scheme_type != URL_SCHEME_UNKNOWN; > const BOOL is_file = data->scheme_type == URL_SCHEME_FILE; > const BOOL is_mk = data->scheme_type == URL_SCHEME_MK; > + const BOOL is_javascript = data->scheme_type == URL_SCHEME_JAVASCRIPT; > + const BOOL is_mailto = data->scheme_type == URL_SCHEME_MAILTO; > + DWORD offset = 0; > > if(!data->path) { > uri->path_start = -1; > @@ -2996,7 +2999,7 @@ static BOOL canonicalize_path_opaque(const parse_data *data, Uri *uri, DWORD fla > } > > /* For javascript: URIs, simply copy path part without any canonicalization */ > - if(data->scheme_type == URL_SCHEME_JAVASCRIPT) { > + if(is_javascript) { > if(!computeOnly) > memcpy(uri->canon_uri+uri->canon_len, data->path, data->path_len*sizeof(WCHAR)); > uri->path_len = data->path_len; > @@ -3007,7 +3010,11 @@ static BOOL canonicalize_path_opaque(const parse_data *data, Uri *uri, DWORD fla > /* Windows doesn't allow a "//" to appear after the scheme > * of a URI, if it's an opaque URI. > */ > - if(data->scheme && *(data->path) == '/' && *(data->path+1) == '/') { > + if (is_mailto && *(data->path) == '/' && *(data->path+1) == '/') { > + if (!*(data->path+2)) uri->path_start = -1; > + offset += 2; I think this should be done in parsing phrase, not during canonicalization. Jacek

March 27, 2013

Re: comctl32: Make an attempt to update notification handle

by Nikolay Sivov

On 3/27/2013 00:10, Daniel Jeliński wrote: > 2013/3/26 Nikolay Sivov <nsivov(a)codeweavers.com>: >> Is there any reliable way to test send mouse messages? Cause it looks like >> if I send WM_LBUTTONDOWN directly it doesn't trigger tab selection code on >> windows, but works in wine. > Do you want to figure out the correct sequence of messages or write > tests for wine's test suite? I need a test that actually works. > Either way, it's a good idea to start > with winetricks comctl32. After that WINEDEBUG=+message gives you all > information you may need. That's a really questionable thing to do cause you'll be looking at native module internals, tracing what it does. I think it should be avoided. > Also some message handlers use mouse coordinates from lParam, while > others seem to use either GetCursorPos or GetMessagePos, so setting > cursor position may be necessary. I didn't think about actually positioning cursor, that could help, thanks. > > Regards, > Daniel

March 27, 2013

Re: iads.idl: Add missing interfaces

by Nikolay Sivov

On 3/27/2013 09:05, Alistair Leslie-Hughes wrote: > Hi, > > > Changelog: > iads.idl: Add missing interfaces > > > Best Regards > Alistair Leslie-Hughes > +typedef enum > +{ > + ADS_SEARCHPREF_ASYNCHRONOUS = 0, > + ADS_SEARCHPREF_DEREF_ALIASES = 1, You don't need to do it manually, widl will generate values for each entry.

March 27, 2013

Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight

by Austin English

On Tue, Mar 26, 2013 at 6:04 PM, Michael Mc Donnell <michael(a)mcdonnell.dk> wrote: > On Tue, Mar 26, 2013 at 7:07 PM, Nozomi Kodama <nozomi.kodama(a)yahoo.com> > wrote: >> >> Why are they not committed yet? > > > Alexandre is on vacation so that might explain why :-) Well, he was last week, back now, but there is a backlog of patches to go through.. Patches were committed today/yesterday: http://source.winehq.org/git/wine.git/shortlog -- -Austin

March 27, 2013

Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight

by Michael Mc Donnell

On Tue, Mar 26, 2013 at 7:07 PM, Nozomi Kodama <nozomi.kodama(a)yahoo.com>wrote: > Why are they not committed yet? > Alexandre is on vacation so that might explain why :-)

March 27, 2013

RE: Linker error when improving GetLargestConsoleWindowSize

by Hugh McMaster

Hi Ken, Thank you for your explanation. I now have a far better understanding of the relationship between kernel32 and user32. The information was actually on MSDN, but I didn't really pay too much attention to the prerequisites for using calls such as SystemParametersInfo. I'm now going to substitute SystemParametersInfo with display information supplied by X11. So far, this method is working fine. The 'make' function has built and linked kernel32.dll with no errors or warnings. Hugh -----Original Message----- From: Ken Thomases [mailto:ken(a)codeweavers.com] Sent: Tuesday, 26 March 2013 1:54 PM To: Hugh McMaster Cc: wine-devel(a)winehq.org Subject: Re: Linker error when improving GetLargestConsoleWindowSize On Mar 25, 2013, at 8:51 PM, Hugh McMaster wrote: > I've written a function to determine the largest possible screen buffer that wineconsole is able to display. This function replaces the hard-coded constants listed in both instances of GetLargestConsoleWindowSize in dlls/kernel32/console.c. > > [snip] > #include <windows.h> > #include <winuser.h> > > [snip] > > COORD GetLargestConsoleWindowSize_helper(HANDLE hConsoleOutput) { > RECT workarea; > COORD console_fontsize; > COORD max_console; > > SystemParametersInfoA(SPI_GETWORKAREA, 0, &workarea, 0); > console_fontsize = GetConsoleFontSize(hConsoleOutput, 0); > > max_console.X = (workarea.right / console_fontsize.X) - 6; > max_console.Y = (workarea.bottom / console_fontsize.Y) - 6; > > return max_console; > } > > While dlls/kernel32/console.c compiles, I'm having trouble with the final compile-time linking. It seems that SystemParametersInfoA is an undefined reference, but windows.h and winuser.h are included in console.c. This won't be able to work. The linker error is telling you, effectively, that you're not importing user32. However, kernel32 can't import user32. Kernel32 is at a lower level than user32. And a console program especially can't rely on user32 being loaded and initialized to satisfy the request you're making. Regards, Ken

March 26, 2013

Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight

by Nozomi Kodama

Why are they not committed yet? ________________________________ From : Matteo Bruni <matteo.mystral(a)gmail.com> Object : Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight 2013/3/15 Nozomi Kodama <nozomi.kodama(a)yahoo.com>: > Patch series is fine to me.

March 26, 2013

Re: Linker error when improving GetLargestConsoleWindowSize

by Eric Pouech

>This won't be able to work. The linker error is telling you, effectively, that you're not importing user32. However, kernel32 can't import user32. Kernel32 is at a lower level than user32. And a console program especially can't rely on user32 being loaded and initialized to satisfy the request you're making. Regards, Ken actually the correct fix would be to transfer this computation to wineconsole (which already imports user32), but thats' not a really trivial task A+

March 26, 2013

Re: comctl32: Make an attempt to update notification handle

by Daniel Jeliński

2013/3/26 Nikolay Sivov <nsivov(a)codeweavers.com>: > Is there any reliable way to test send mouse messages? Cause it looks like > if I send WM_LBUTTONDOWN directly it doesn't trigger tab selection code on > windows, but works in wine. Do you want to figure out the correct sequence of messages or write tests for wine's test suite? Either way, it's a good idea to start with winetricks comctl32. After that WINEDEBUG=+message gives you all information you may need. First thing you may want to check is if there's a modal message loop involved. Run the application with +message and click. If the last line in console is '...WM_LBUTTONDOWN returned', you're looking at a message loop. If it's WM_LBUTTONUP, it's probably standard handling. Keep in mind that some window styles may affect message handling (e.g. treeview handles mouse messages differently when TVS_DISABLEDRAGDROP is specified) If you want to write tests, you may be able to use SendMessage. It's much less work than SendInput or mouse_event, and is more reliable as BlockInput is not implemented. I had some degree of success with SendMessage when testing ListView and Treeview, but my tests never made it into git. Also some message handlers use mouse coordinates from lParam, while others seem to use either GetCursorPos or GetMessagePos, so setting cursor position may be necessary. Regards, Daniel

March 26, 2013

Re: msvcr110: forward __crtSetUnhandledExceptionFilter to kernel32.SetUnhandledExceptionFilter

by Alexandre Julliard

Austin English <austinenglish(a)gmail.com> writes: > diff --git a/dlls/msvcr110/msvcr110.spec b/dlls/msvcr110/msvcr110.spec > index 4a94dbe..babfc67 100644 > --- a/dlls/msvcr110/msvcr110.spec > +++ b/dlls/msvcr110/msvcr110.spec > @@ -928,7 +928,7 @@ > @ stub -arch=i386,win64 __crtLCMapStringEx > @ cdecl __crtLCMapStringW(long long wstr long ptr long long long) msvcrt.__crtLCMapStringW > @ stub -arch=i386,win64 __crtSetThreadStackGuarantee > -@ stub __crtSetUnhandledExceptionFilter > +@ stdcall __crtSetUnhandledExceptionFilter(ptr) kernel32.SetUnhandledExceptionFilter A quick test shows that the crt functions are cdecl. -- Alexandre Julliard julliard(a)winehq.org

March 26, 2013

Re: Mac driver status: ready for contributions

by Francois Gouget

On Mon, 18 Mar 2013, Ken Thomases wrote: [...] > Currently, you have to set a registry key to enable the Mac driver: > > [HKEY_CURRENT_USER\Software\Wine\Drivers] > "Graphics"="mac,x11" An issue I ran into with the registry approach is that it causes Wine to use X11 to show the 'Updating the Wine configuration' dialog during the WINEPREFIX creation. It's pretty misleading and could easily give the wrong impression! Wine does shift to using the Mac driver once regedit has had a chance to be run. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ I haven't lost my mind, it's backed up on tape around here somewhere...

March 26, 2013

Re: gdi32: Printer drivers don't use the character extra spacing if lpdx is supplied.

by Huw Davies

On Tue, Mar 26, 2013 at 04:50:03PM +0100, Alexandre Julliard wrote: > Ah OK, I guess that makes it a PITA to test... Some kind of print-scan magic comes to mind ;-)

March 26, 2013

Re: gdi32: Printer drivers don't use the character extra spacing if lpdx is supplied.

by Alexandre Julliard

Huw Davies <huw(a)codeweavers.com> writes: > On Tue, Mar 26, 2013 at 04:30:14PM +0100, Alexandre Julliard wrote: >> Huw Davies <huw(a)codeweavers.com> writes: >> >> > Ideally this would be done in wineps.drv, but by that point the conversion to device units would cause rounding errors. >> >> What about GetTextExtent functions? Do they need this too? Some test >> cases would be nice. > > Note this only applies when lpDx != NULL, when it's NULL then > char_extra is still applied. So, yes the current GetTextExtent > behaviour is correct. Ah OK, I guess that makes it a PITA to test... -- Alexandre Julliard julliard(a)winehq.org

March 26, 2013

Re: gdi32: Printer drivers don't use the character extra spacing if lpdx is supplied.

by Huw Davies

On Tue, Mar 26, 2013 at 04:30:14PM +0100, Alexandre Julliard wrote: > Huw Davies <huw(a)codeweavers.com> writes: > > > Ideally this would be done in wineps.drv, but by that point the conversion to device units would cause rounding errors. > > What about GetTextExtent functions? Do they need this too? Some test > cases would be nice. Note this only applies when lpDx != NULL, when it's NULL then char_extra is still applied. So, yes the current GetTextExtent behaviour is correct. Huw.

March 26, 2013

Re: gdi32: Printer drivers don't use the character extra spacing if lpdx is supplied.

by Alexandre Julliard

Huw Davies <huw(a)codeweavers.com> writes: > Ideally this would be done in wineps.drv, but by that point the conversion to device units would cause rounding errors. What about GetTextExtent functions? Do they need this too? Some test cases would be nice. -- Alexandre Julliard julliard(a)winehq.org

March 26, 2013

Re: comctl32: Make an attempt to update notification handle

by Nikolay Sivov

On 3/25/2013 07:48, Dmitry Timoshkov wrote: >>>>> Why not just call GetParent() every time when a >>>>> notification is being sent? >>>> Because it won't work if you create a control without WS_CHILD. >>> How is that different from the WM_WINDOWPOSCHANGING handler? >> How what is different? There's a check for null parent as you can see. > Why the same can't be done at the time a notification needs to be sent? > There are many places (starting from user32/WM_PARENTNOTIFY) which send > notifications that way, what's so special about tab control? Unfortunately it's more complicated than that. For example notifications sent after TCM_SETCURFOCUS are delivered to original window, but if you do any mouse interaction it appears to notify new parent. So SetParent() leaves it in a kind of broken state. So we can't always use parent window, but for some notification cases valid GetParent() value should be used as in examples you mentioned. Is there any reliable way to test send mouse messages? Cause it looks like if I send WM_LBUTTONDOWN directly it doesn't trigger tab selection code on windows, but works in wine. Thanks for review.

March 26, 2013

Re: explorer: Create desktop window hidden and only show it if it wins the race.

by Ken Thomases

On Mar 26, 2013, at 3:20 AM, Dmitry Timoshkov wrote: > Ken Thomases <ken(a)codeweavers.com> wrote: > >> + ShowWindow( hwnd, SW_SHOW ); >> SetWindowLongPtrW( hwnd, GWLP_WNDPROC, (LONG_PTR)desktop_wnd_proc ); >> SendMessageW( hwnd, WM_SETICON, ICON_BIG, (LPARAM)LoadIconW( 0, MAKEINTRESOURCEW(OIC_WINLOGO))); >> if (name) set_desktop_window_title( hwnd, name ); > > You can postpone calling ShowWindow() after setting window text and icon, > since they are drawn as part of a non-client area, and changing them > forces another set of redraws. Good idea. Thanks. -Ken

March 26, 2013

Re: explorer: Create desktop window hidden and only show it if it wins the race.

by Dmitry Timoshkov

Ken Thomases <ken(a)codeweavers.com> wrote: > + ShowWindow( hwnd, SW_SHOW ); > SetWindowLongPtrW( hwnd, GWLP_WNDPROC, (LONG_PTR)desktop_wnd_proc ); > SendMessageW( hwnd, WM_SETICON, ICON_BIG, (LPARAM)LoadIconW( 0, MAKEINTRESOURCEW(OIC_WINLOGO))); > if (name) set_desktop_window_title( hwnd, name ); You can postpone calling ShowWindow() after setting window text and icon, since they are drawn as part of a non-client area, and changing them forces another set of redraws. -- Dmitry.

March 26, 2013

Mac driver screen flicker on launch (Was: Re: [1/3] Make mac driver the default on OS X)

by Ken Thomases

On Mar 8, 2013, at 6:20 AM, <Joerg-Cyril.Hoehle(a)t-systems.com> <Joerg-Cyril.Hoehle(a)t-systems.com> wrote: > I had a very annoying experience: when I started winemac the first > time, the whole screen turned black for a fraction of a second. That > was very unwelcome and reminded me of the infamous Intel-XOrg screen > flickering issue that plagued many Linux distros a few years ago :-( I have tracked this down. Unless explorer.exe is launched explicitly, it is launched on demand. Inside user32.GetDesktopWindow() there's a check if the desktop window exists. If it doesn't, then explorer.exe is launched. There is a race, of course, if multiple processes or threads call GetDesktopWindow() at roughly the same time before the desktop window has been created. All of them start explorer.exe. Explorer.exe creates a window that it intends to act as the desktop window. It then checks if that window actually became the desktop window. If it doesn't, it figures some other instance of explorer.exe won the race, destroys the window, and exits. Only one of the explorer.exe instances survives and it owns the real desktop window. The flicker is the prospective desktop window of the losers. The windows are created visible, so they are briefly shown before being destroyed. (The real desktop window is also created visible but it doesn't usually show unless virtual desktop is enabled under X11. That's because both the Mac and X11 drivers have checks for the desktop window and treat it specially. But only the real desktop window, from the winning explorer.exe, qualifies for that special treatment. The windows from the losers are just normal windows.) I think the solution is to have explorer.exe create its prospective desktop window hidden and only show it if it wins the race. I'll submit a patch shortly. -Ken

March 26, 2013

Re: Wiki is down

by Dimi Paun

Should be up now, the RAID controller on the box died. We'll be moving it to a better box soon, stay tuned. Dimi. On Mon, Mar 25, 2013 at 10:06 AM, Dimi Paun <dimi(a)lattica.com> wrote: > Working on it guys, I'll keep you posted. > > Dimi. > > On Mon, Mar 25, 2013 at 7:18 AM, Francois Gouget <fgouget(a)free.fr> wrote: > >> On Sun, 24 Mar 2013, Rosanne DiMesio wrote: >> >> > I'm getting an "Internal Server Error" message when I try to access the >> wiki. >> >> Cc-ing in case you missed this email (the Wiki is still down :-( ). >> >> >> -- >> Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ >> Dieu dit: "M-x Lumière". Et la lumière fut. > > > > > -- > Dimi Paun <dimi(a)lattica.com> > Lattica, Inc. > > -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.

March 26, 2013

Re: Linker error when improving GetLargestConsoleWindowSize

by Ken Thomases

On Mar 25, 2013, at 8:51 PM, Hugh McMaster wrote: > I've written a function to determine the largest possible screen buffer that wineconsole is able to display. This function replaces the hard-coded constants listed in both instances of GetLargestConsoleWindowSize in dlls/kernel32/console.c. > > [snip] > #include <windows.h> > #include <winuser.h> > > [snip] > > COORD GetLargestConsoleWindowSize_helper(HANDLE hConsoleOutput) > { > RECT workarea; > COORD console_fontsize; > COORD max_console; > > SystemParametersInfoA(SPI_GETWORKAREA, 0, &workarea, 0); > console_fontsize = GetConsoleFontSize(hConsoleOutput, 0); > > max_console.X = (workarea.right / console_fontsize.X) - 6; > max_console.Y = (workarea.bottom / console_fontsize.Y) - 6; > > return max_console; > } > > While dlls/kernel32/console.c compiles, I'm having trouble with the final compile-time linking. It seems that SystemParametersInfoA is an undefined reference, but windows.h and winuser.h are included in console.c. This won't be able to work. The linker error is telling you, effectively, that you're not importing user32. However, kernel32 can't import user32. Kernel32 is at a lower level than user32. And a console program especially can't rely on user32 being loaded and initialized to satisfy the request you're making. Regards, Ken

March 26, 2013

Linker error when improving GetLargestConsoleWindowSize

by Hugh McMaster

Hi, I've written a function to determine the largest possible screen buffer that wineconsole is able to display. This function replaces the hard-coded constants listed in both instances of GetLargestConsoleWindowSize in dlls/kernel32/console.c. [snip] #include <windows.h> #include <winuser.h> [snip] COORD GetLargestConsoleWindowSize_helper(HANDLE hConsoleOutput) { RECT workarea; COORD console_fontsize; COORD max_console; SystemParametersInfoA(SPI_GETWORKAREA, 0, &workarea, 0); console_fontsize = GetConsoleFontSize(hConsoleOutput, 0); max_console.X = (workarea.right / console_fontsize.X) - 6; max_console.Y = (workarea.bottom / console_fontsize.Y) - 6; return max_console; } While dlls/kernel32/console.c compiles, I'm having trouble with the final compile-time linking. It seems that SystemParametersInfoA is an undefined reference, but windows.h and winuser.h are included in console.c. This is the terminal output: make[1]: Entering directory `/home/hugh/wine-1.5.26/dlls/kernel32' [snip] ../../tools/winegcc/winegcc -B../../tools/winebuild --sysroot=../.. -fasynchronous-unwind-tables -shared ./kernel32.spec actctx.o atom.o change.o comm.o computername.o console.o cpu.o debugger.o editline.o environ.o except.o fiber.o file.o format_msg.o heap.o kernel_main.o lcformat.o locale.o lzexpand.o module.o nameprep.o oldconfig.o path.o powermgnt.o process.o profile.o resource.o string.o sync.o tape.o term.o thread.o time.o toolhelp.o version.o virtual.o volume.o wer.o locale_rc.res version.res winerror.res -nodefaultlibs -Wb,-F,KERNEL32.dll -Wl,--image-base,0x7b800000 -o kernel32.dll.so -lwinecrt0 -lntdll ../../libs/port/libwine_port.a console.o: In function `GetLargestConsoleWindowSize_helper': /home/hugh/wine-1.5.26/dlls/kernel32/console.c:1377: undefined reference to `SystemParametersInfoA' /usr/bin/ld: kernel32.dll.so: hidden symbol `SystemParametersInfoA' isn't defined /usr/bin/ld: final link failed: Bad value collect2: error: ld returned 1 exit status winegcc: gcc failed make[1]: *** [kernel32.dll.so] Error 2 Unfortunately, my knowledge of C is terrible, so I'm not sure what to do from here. If someone could help me solve this, I'd appreciate it. If it helps, I'm able to use winegcc to build a simple Windows executable using SystemParametersInfo without any problems. Thank you in advance, Hugh

March 26, 2013

Re: Wiki is down

by Dimi Paun

Working on it guys, I'll keep you posted. Dimi. On Mon, Mar 25, 2013 at 7:18 AM, Francois Gouget <fgouget(a)free.fr> wrote: > On Sun, 24 Mar 2013, Rosanne DiMesio wrote: > > > I'm getting an "Internal Server Error" message when I try to access the > wiki. > > Cc-ing in case you missed this email (the Wiki is still down :-( ). > > > -- > Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ > Dieu dit: "M-x Lumière". Et la lumière fut. -- Dimi Paun <dimi(a)lattica.com> Lattica, Inc.

March 25, 2013

Re: Wiki is down

by Francois Gouget

On Sun, 24 Mar 2013, Rosanne DiMesio wrote: > I'm getting an "Internal Server Error" message when I try to access the wiki. Cc-ing in case you missed this email (the Wiki is still down :-( ). -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Dieu dit: "M-x Lumière". Et la lumière fut.

March 25, 2013

Re: comctl32: Make an attempt to update notification handle

by Dmitry Timoshkov

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > >>>> + case WM_WINDOWPOSCHANGING: > >>>> + { > >>>> + HWND parent = GetParent(hwnd); > >>>> + if (parent) infoPtr->hwndNotify = parent; > >>>> + return DefWindowProcW(hwnd, uMsg, wParam, lParam); > >>>> + } > >>> What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING > >>> to the original proc? > >> Then it will fail just like it does currently, and will work for the > >> rest of the applications. > > Is it really supposed to fail? > That's a different problem. No, it's the same problem of not tested behaviour. > >>> Why not just call GetParent() every time when a > >>> notification is being sent? > >> Because it won't work if you create a control without WS_CHILD. > > How is that different from the WM_WINDOWPOSCHANGING handler? > How what is different? There's a check for null parent as you can see. Why the same can't be done at the time a notification needs to be sent? There are many places (starting from user32/WM_PARENTNOTIFY) which send notifications that way, what's so special about tab control? -- Dmitry.

March 25, 2013

Re: comctl32: Make an attempt to update notification handle

by Nikolay Sivov

On 3/25/2013 07:34, Dmitry Timoshkov wrote: > Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > >>>> + case WM_WINDOWPOSCHANGING: >>>> + { >>>> + HWND parent = GetParent(hwnd); >>>> + if (parent) infoPtr->hwndNotify = parent; >>>> + return DefWindowProcW(hwnd, uMsg, wParam, lParam); >>>> + } >>> What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING >>> to the original proc? >> Then it will fail just like it does currently, and will work for the >> rest of the applications. > Is it really supposed to fail? That's a different problem. > >>> Why not just call GetParent() every time when a >>> notification is being sent? >> Because it won't work if you create a control without WS_CHILD. > How is that different from the WM_WINDOWPOSCHANGING handler? How what is different? There's a check for null parent as you can see.

March 25, 2013

Re: comctl32: Make an attempt to update notification handle

by Dmitry Timoshkov

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > >> + case WM_WINDOWPOSCHANGING: > >> + { > >> + HWND parent = GetParent(hwnd); > >> + if (parent) infoPtr->hwndNotify = parent; > >> + return DefWindowProcW(hwnd, uMsg, wParam, lParam); > >> + } > > What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING > > to the original proc? > Then it will fail just like it does currently, and will work for the > rest of the applications. Is it really supposed to fail? > > Why not just call GetParent() every time when a > > notification is being sent? > Because it won't work if you create a control without WS_CHILD. How is that different from the WM_WINDOWPOSCHANGING handler? -- Dmitry.

March 25, 2013

Re: comctl32: Make an attempt to update notification handle

by Nikolay Sivov

On 3/25/2013 07:24, Dmitry Timoshkov wrote: > Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > >> + case WM_WINDOWPOSCHANGING: >> + { >> + HWND parent = GetParent(hwnd); >> + if (parent) infoPtr->hwndNotify = parent; >> + return DefWindowProcW(hwnd, uMsg, wParam, lParam); >> + } > What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING > to the original proc? Then it will fail just like it does currently, and will work for the rest of the applications. > Why not just call GetParent() every time when a > notification is being sent? Because it won't work if you create a control without WS_CHILD.

March 25, 2013

Re: comctl32: Make an attempt to update notification handle

by Dmitry Timoshkov

Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > + case WM_WINDOWPOSCHANGING: > + { > + HWND parent = GetParent(hwnd); > + if (parent) infoPtr->hwndNotify = parent; > + return DefWindowProcW(hwnd, uMsg, wParam, lParam); > + } What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING to the original proc? Why not just call GetParent() every time when a notification is being sent? -- Dmitry.

March 25, 2013

Re: [4/5] hlink: Implement IHlinkBrowseContext_GetHlink()

by Marvin

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=24822 Your paranoid android. === W2KPROSP4 (32 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005 === W2K3R2SESP2 (32 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005 === W2K8SE (32 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005 === W7PROX64 (32 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005 === TEST64_W7SP1 (32 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005 === W7PROX64 (64 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005 === TEST64_W7SP1 (64 bit browse_ctx) === browse_ctx.c:61: Test failed: got 0x80004005 browse_ctx.c:64: Test failed: got 0x80004005

March 25, 2013

Re: ntdll: Improve stub for FileFsAttributeInformation in NtQueryVolumeInformationFile

by Marvin

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=24819 Your paranoid android. === W2KPROSP4 (32 bit file) === file.c:1675: Test failed: got 8 === W2K3R2SESP2 (32 bit file) === file.c:1675: Test failed: got 8 === W2K8SE (32 bit file) === file.c:1675: Test failed: got 8 === W7PROX64 (32 bit file) === file.c:1675: Test failed: got 8 === TEST64_W7SP1 (32 bit file) === file.c:1675: Test failed: got 8 === W7PROX64 (64 bit file) === file.c:1675: Test failed: got 8 === TEST64_W7SP1 (64 bit file) === file.c:1675: Test failed: got 8

March 24, 2013

Wiki is down

by Rosanne DiMesio

I'm getting an "Internal Server Error" message when I try to access the wiki. -- Rosanne DiMesio <dimesio(a)earthlink.net>

March 24, 2013

Re: winemac: Calculate surface rect correctly in macdrv_UpdateLayeredWindow().

by Ken Thomases

On Mar 22, 2013, at 1:21 AM, Ken Thomases wrote: > --- > dlls/winemac.drv/window.c | 33 ++++++++++++++++----------------- > 1 files changed, 16 insertions(+), 17 deletions(-) > > <0001-winemac-Calculate-surface-rect-correctly-in-macdrv_U.patch> Hmm. On further consideration, I'm not sure this is right. I think the intent was for the window surface of a ULW window to have a full surface even if it's partially off-screen. So, perhaps macdrv_WindowPosChanging() needs to use the same calculation as macdrv_UpdateLayeredWindow() for that kind of window so the surface rect doesn't keep ping-ponging. Also, the same issue exists in the X11 driver. I'll have to discuss with Alexandre when he gets back. -Ken

March 22, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by Nikolay Sivov

On 3/22/2013 10:08, Ken Thomases wrote: > On Mar 21, 2013, at 3:41 PM, C.W. Betts wrote: > >> On Mar 18, 2013, at 3:22 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >> >>> On Mar 18, 2013, at 4:04 PM, Charles Davis wrote: >>> >>>> At this point, though, I'm wondering if it wouldn't just be easier to have Explorer draw the balloons itself à la Windows XP. >>> Probably. I also considered using an NSPopover anchored to the status item. Again, that's 10.7+ functionality. >> We could implement it and do compile- and runtime checks to make sure that the functions aren't called in unsupported OS X versions. However, this would probably add fragmentation, and I doubt Alexandre would be happy with this. > I think he'd be OK with some variant of this. He might not want two implementations, one using NSPopover on 10.7+ and another for 10.6. I don't know how he'd feel about notification balloons only being implemented on 10.7+. > > Although I'm not sure how important notification balloons are in the grand scheme of things, anyway. I guess users will let us know if they miss them (unless somebody is inspired to take on that particular corner of unimplemented functionality beforehand). I might be wrong but I think it was some .net installer that wanted user to click on this balloon popup to complete, on the end of installation process it was just waiting for such user input. > Cheers, > Ken > > > >

March 22, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by Ken Thomases

On Mar 21, 2013, at 3:41 PM, C.W. Betts wrote: > On Mar 18, 2013, at 3:22 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > >> On Mar 18, 2013, at 4:04 PM, Charles Davis wrote: >> >>> At this point, though, I'm wondering if it wouldn't just be easier to have Explorer draw the balloons itself à la Windows XP. >> >> Probably. I also considered using an NSPopover anchored to the status item. Again, that's 10.7+ functionality. > > We could implement it and do compile- and runtime checks to make sure that the functions aren't called in unsupported OS X versions. However, this would probably add fragmentation, and I doubt Alexandre would be happy with this. I think he'd be OK with some variant of this. He might not want two implementations, one using NSPopover on 10.7+ and another for 10.6. I don't know how he'd feel about notification balloons only being implemented on 10.7+. Although I'm not sure how important notification balloons are in the grand scheme of things, anyway. I guess users will let us know if they miss them (unless somebody is inspired to take on that particular corner of unimplemented functionality beforehand). Cheers, Ken

March 22, 2013

Re: Parameterizing the places Wine suggests people report bugs

by Austin English

On Mon, Mar 18, 2013 at 5:18 PM, Josh DuBois <duboisj(a)codeweavers.com> wrote: > Hi all, > > There are a couple of places wine suggests that people report bugs to > wine-devel. The crash reporting dialog in winedbg suggests this, and a > couple of places in code refer to wine-devel(a)winehq.org in error messages. I assume you meant http://bugs.winehq.org for the winedbg dialog, since that's what I get/the po files refer to: "Report-Msgid-Bugs-To: http://bugs.winehq.org\n" > The configure script has a PACKAGE_BUGREPORT macro which could be used to > allow someone building wine to redirect those errors somewhere else. > > At least for wine's crash dialog, an alternate approach for a repackager > to redirect bug reports would be to read the url / email address for where > to send bugs from the registry. > > Would Wine accept a patch for making winedbg read the 'destination' for > bug reports from the registry or some other runtime variable? Or would the > project prefer to have all the bug reports routed directly to winehq, so > that a repackager who wants to re-route bugs would have to build that > facility on its own? I can't speak for Alexandre, of course, but speaking with my bugzilla hat on, if a packager wants to override that, I don't see the harm. I'd rather see wine bugs reported upstream (i.e., http://bugs.winehq.org) but if the packager wants to override that, I assume they have a good reason (building wine with custom/hacky patches, changing default registry settings, etc.). How many packagers will actually use that feature though, and how many users would notice the difference, is the ultimate question, of course :). -- -Austin

March 22, 2013

Re: gdi32: Copy gamma ramp validation from winex11 to make it driver independent

by Dmitry Timoshkov

André Hentschel <nerv(a)dawncrow.de> wrote: > - this was done for the mac driver > - i tried to follow the file-style, except of the original variable naming The ERRs should be downgraded to WARNs or even TRACEs, there is no point to loudly complain on application input. -- Dmitry.

March 22, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by C.W. Betts

On Mar 18, 2013, at 3:22 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > On Mar 18, 2013, at 4:04 PM, Charles Davis wrote: > >> On Mar 18, 2013, at 2:23 PM, Ken Thomases wrote: >> >>> On Mar 18, 2013, at 2:15 PM, Ken Thomases wrote: >>> >>>> On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote: >>>> >>>>> On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >>>>> >>>>>> Doesn't support right-clicks, mouse moves, or notification balloons. >>>>> Notification balloons can probably be done using either Notification Center or Growl >>>> >>>> Yeah, I was thinking that the notification center is the right way to go. >>> >>> Josh reminds me that only code-signed apps can use the Notification Center, so that may not be very useful. *sigh* >> So what? [...] >> >> Though I wonder if it's a good idea to ask users to create a self-signed certificate just so Wine can use the Notification Center... > > You answered your own question. ;) > > >> At this point, though, I'm wondering if it wouldn't just be easier to have Explorer draw the balloons itself à la Windows XP. > > Probably. I also considered using an NSPopover anchored to the status item. Again, that's 10.7+ functionality. We could implement it and do compile- and runtime checks to make sure that the functions aren't called in unsupported OS X versions. However, this would probably add fragmentation, and I doubt Alexandre would be happy with this.

March 21, 2013

Re: [GSoC] My proposal for GSoC 2013

by Gediminas Jakutis

Hello again! Thank You all for the feedback! >There's a bit more info at >http://wiki.winehq.org/FAQ#head-dba86ae589dca54a116a3fc21f6237af45e8119c. That's odd - I could've sworn this was an environment variable which had the same limitations as using winecfg! My memory must be playing tricks on me... And yes, now that I tested it again, I see it really does the job. Except for not working with steam games, but that is steam's fault, not Wine's. >Though you did start early, so you have plenty of time to find other ideas Indeed... But now that I have to think of something else - what general areas in Wine are most welcome for GSoC ideas? I Looked over the GSoC Wine's wiki page, but I do not know how much I can trust it. That is because after GSoC 2012, it was only edited once. And the edit comment says: > remove some stuff that's done (still quite a bit of cleanup left).. Thank You for Your time! Gediminas Jakutis -- LDK Varčiai www.varciai.lt

March 21, 2013

Re: [GSoC] My proposal for GSoC 2013

by Austin English

On Wed, Mar 20, 2013 at 11:35 AM, Gediminas Jakutis <gediminas(a)varciai.lt> wrote: > Hello! > For GSoC, I am suggesting my own idea. In case this idea is not good, > I am open to changing it, thinking of a new one, or adopting one. > My idea: I have noticed that Wine's virtual desktop feature is very > limited - if the first program on a prefix starts without a virtual > desktop, all programs launched during the same session also start > without a VD, disregarding any VD settings and/or requests with > environment variables. And vice versa - if the first program starts > inside a VD, all other programs in that sessions start in the VD. > Often, this can be worked around by using separate Wine prefixes. Yet > sometimes, this workaround is not possible. A good example: steam and > games that only run properly in a VD. This forces the user to start > steam in a VD and makes steam itself unusable while a game is running, > especially if the game does not support the steam overlay. I would > like to fix this limitation and make it possible to properly choose to > run different programs on one prefix with or without a VD regardless > what was chosen to programs already running in that prefix. I can > already see this requiring changes in the winecfg utility, by the way. In addition to Vincent's comments, note that you _can_ set up virtual desktops for certain apps but not others within a single WINEPREFIX (though Steam may not like this, I haven't tried it myself). There's a bit more info at http://wiki.winehq.org/FAQ#head-dba86ae589dca54a116a3fc21f6237af45e8119c. This has been in Wine for quite a while...a significant part of Summer of Code is researching potential ideas before proposing them. Though you did start early, so you have plenty of time to find other ideas :). -- -Austin

March 21, 2013

Re: winemac: Implement GetDeviceGammaRamp() and SetDeviceGammaRamp().

by Ken Thomases

On Mar 20, 2013, at 1:33 PM, André Hentschel wrote: > Am 18.03.2013 04:41, schrieb Ken Thomases: >> --- >> dlls/winemac.drv/display.c | 131 ++++++++++++++++++++++++++++++++++++++++++++ >> dlls/winemac.drv/gdi.c | 4 +- >> dlls/winemac.drv/macdrv.h | 2 + >> 3 files changed, 135 insertions(+), 2 deletions(-) > > Hi, > wrt macdrv_SetDeviceGammaRamp, the X11 driver has some validation in xvidmode.c->ComputeGammaFromRamp() because windows doesn't allow every ramp. > It might happen that apps try bad ramps and the screen simply gets dark without validation. Ah, interesting. I missed that. > I'll see if i can copy/move the validation to gdi32 where the driver function normally get's called so that the validation is driver independent. Thanks! -Ken

March 20, 2013

Re: [GSoC] My proposal for GSoC 2013

by Vincent Povirk

I think the reasons for the limitation on the virtual desktop setting are more ideological than technical. It used to work the way you say, but then Wine's desktop management was overhauled so that: * Explorer.exe manages all desktop windows, including the virtual desktop. I believe this is required for multiple processes to exist in the same virtual desktop window. * Wine's virtual desktops now use the window station/desktop API's (which also exist on Windows). In theory, Windows programs can use this to inspect all of the available desktops, interact with windows on other desktops, and control the desktop window of child processes. But it also means programs on different desktops are isolated from each other unless they're programmed with desktops in mind. * Processes inherit desktops from parent processes rather than creating their own. This is technically necessary because of the previous point about isolating desktops. If a program spawns a child process that ends up in a different desktop, the child may not be able to communicate with the parent, and you end up with a Wine bug. You could perhaps argue that the concept of a virtual desktop window and the win32 concept of a desktop should be separated, and it should be possible for a process to exist in a separate desktop window while being in the same win32 desktop as processes that exist in a root window. That sort of thing can currently happen when Wine is run from two different X sessions, such as a local desktop and remote X over ssh. Taking steam and some fullscreen game as an example, some weird things happen when the fullscreen game is on the same desktop as steam but its windows are in a virtual desktop. For one, the two processes don't see the same screen resolution. If the steam window gains focus, the fullscreen game necessarily loses focus (because only one process per desktop can be focused), and that may cause the fullscreen game to attempt to minimize itself. The challenge of making this work is to come up with a system that is consistent, a correct implementation of the win32 api, enables the use case you want while not breaking anything that works now, and can be implemented cleanly. If you can manage all of those things, you can probably get your change into Wine, but it's a really hard problem and might not even be possible.

March 20, 2013

[GSoC] My proposal for GSoC 2013

by Gediminas Jakutis

Hello! My name is Gediminas Jakutis. I study Informatics Engineering [bachelor / undergraduate] at Kaunas University of Technology, second year. I wish to try out GSoC for the first time by helping to improve Wine. My primary programming language is C. I am self-taught in C, but I believe my knowledge is good enough. I am quite familiar with Wine - I use it and experiment with it for many years now. I am subscribed to and read wine-devel for more than a year. I browse through Wine's source code once in a while out of curiosity. Thus, I have some knowledge of its structure. Sadly, I have not written any patches for Wine yet. Mostly due to lack of confidence. For GSoC, I am suggesting my own idea. In case this idea is not good, I am open to changing it, thinking of a new one, or adopting one. My idea: I have noticed that Wine's virtual desktop feature is very limited - if the first program on a prefix starts without a virtual desktop, all programs launched during the same session also start without a VD, disregarding any VD settings and/or requests with environment variables. And vice versa - if the first program starts inside a VD, all other programs in that sessions start in the VD. Often, this can be worked around by using separate Wine prefixes. Yet sometimes, this workaround is not possible. A good example: steam and games that only run properly in a VD. This forces the user to start steam in a VD and makes steam itself unusable while a game is running, especially if the game does not support the steam overlay. I would like to fix this limitation and make it possible to properly choose to run different programs on one prefix with or without a VD regardless what was chosen to programs already running in that prefix. I can already see this requiring changes in the winecfg utility, by the way. In case this turns out to be too easy of a project, I could implement some way to perform "alt-tabbing" between programs in a VD and/or an optional (enable-able in winecfg) rudimentary taskbar for changing/checking focus. I am very excited about participating in GSoC. And even more excited about starting to contribute to Wine by writing patches! Regardless if I get accepted or not, I wish to make Wine better. Thus, expect me to start submitting patches in the future even if I do not get accepted. ;] Sincerely, Gediminas Jakutis ---- LDK Varčiai www.varciai.lt

March 20, 2013

Re: winemac: Implement GetDeviceGammaRamp() and SetDeviceGammaRamp().

by André Hentschel

Am 18.03.2013 04:41, schrieb Ken Thomases: > --- > dlls/winemac.drv/display.c | 131 ++++++++++++++++++++++++++++++++++++++++++++ > dlls/winemac.drv/gdi.c | 4 +- > dlls/winemac.drv/macdrv.h | 2 + > 3 files changed, 135 insertions(+), 2 deletions(-) Hi, wrt macdrv_SetDeviceGammaRamp, the X11 driver has some validation in xvidmode.c->ComputeGammaFromRamp() because windows doesn't allow every ramp. It might happen that apps try bad ramps and the screen simply gets dark without validation. I'll see if i can copy/move the validation to gdi32 where the driver function normally get's called so that the validation is driver independent. -- Best Regards, André Hentschel

March 20, 2013

Re: [PATCH 2/2] msvcrt: Fixed fgetwc behavior on multibyte characters and unicode files

by Piotr Caban

On 03/20/13 09:41, Akihiro Sagawa wrote: > On Mon, 18 Mar 2013 17:11:04 +0100, Piotr Caban wrote: >> + char mbs[2]; > This should be a constant, like MSVCRT_MB_LEN_MAX. > >> + int len = 0; >> + ch = MSVCRT_fgetc(file); >> + if(ch != MSVCRT_EOF) { >> + mbs[0] = (char)ch; >> + if(MSVCRT_isleadbyte(mbs[0])) { > This doesn't work correctly if char is signed. Because _isleadbyte() > expects the value of unsigned char or EOF as well as other is- functions. > > I wrote some fgetwc() tests. Please refer following page: > https://testbot.winehq.org/JobDetails.pl?Key=24806 I've sent a fixed version. Thank you. Thanks, Piotr

March 20, 2013

Re: [PATCH 2/2] msvcrt: Fixed fgetwc behavior on multibyte characters and unicode files

by Akihiro Sagawa

On Mon, 18 Mar 2013 17:11:04 +0100, Piotr Caban wrote: > + char mbs[2]; This should be a constant, like MSVCRT_MB_LEN_MAX. > + int len = 0; > + ch = MSVCRT_fgetc(file); > + if(ch != MSVCRT_EOF) { > + mbs[0] = (char)ch; > + if(MSVCRT_isleadbyte(mbs[0])) { This doesn't work correctly if char is signed. Because _isleadbyte() expects the value of unsigned char or EOF as well as other is- functions. I wrote some fgetwc() tests. Please refer following page: https://testbot.winehq.org/JobDetails.pl?Key=24806 Akihiro Sagawa

March 20, 2013

Re: [7/10] winemenubuilder: Create a basic Info.plist.

by Per Johansson

On Tue, Mar 19, 2013 at 5:57 AM, Ken Thomases <ken(a)codeweavers.com> wrote: > On Mar 17, 2013, at 12:18 PM, Per Johansson wrote: > >> + namestr = CFStringCreateWithCString(NULL, link_name, CFStringGetSystemEncoding()); > > You should use CFStringCreateWithFileSystemRepresentation() to create CFStrings from POSIX path strings. Fixed. Thanks, -- Per Johansson

March 20, 2013

Re: Mac driver status: ready for contributions

by Ken Thomases

On Mar 19, 2013, at 1:31 AM, Charles Davis wrote: > I have one more of my own to add: > > * Fix crash whenever an app makes a GLU call. Ah, yes, I forgot that one. Thanks for reminding me. > I suspect this is because Mesa's GLU calls Mesa's libGL--which crashes horribly because there's no Mesa/GLX context current. Yes, basically. > I see two ways to fix that. One is to make GLU dispatch go through the driver. This is what I did for CrossOver 12. The problem is that it introduced a requirement that a WGL context be current on the thread to find the driver functions. (Technically, the GLU spec requires a current GL context at the time of any GLU calls, but glu32 doesn't require that and, empirically, it seems that Mesa's library doesn't either. Because some stuff that used to work was broken by CrossOver 12.) > The other is to actually implement glu32 (i.e. not as a forwarder to the system GLU). I'm not sure how AJ will feel about the first one--and I probably won't know until next week. Not thrilled, although willing to entertain it if it can be done in a relatively elegant and robust way. None of my proposals rose to those standards. ;) > The second one... writing a GLU is a lot of work. We could use SGI's (as Mesa does), but SGI GLU is largely written in C++--and I *know* AJ won't like that. Implementing glu32 on top of opengl32 was Alexandre's approach. He was looking for an open-source GLU implementation to use. He could find C implementations for most parts of GLU except the NURBS stuff. We didn't really get any further than that. -Ken

March 19, 2013

Re: Mac driver status: ready for contributions

by Charles Davis

I have one more of my own to add: * Fix crash whenever an app makes a GLU call. I suspect this is because Mesa's GLU calls Mesa's libGL--which crashes horribly because there's no Mesa/GLX context current. I see two ways to fix that. One is to make GLU dispatch go through the driver. The other is to actually implement glu32 (i.e. not as a forwarder to the system GLU). I'm not sure how AJ will feel about the first one--and I probably won't know until next week. The second one... writing a GLU is a lot of work. We could use SGI's (as Mesa does), but SGI GLU is largely written in C++--and I *know* AJ won't like that. There's probably something I'm missing here, though. I'd be happy to have that pointed out to me. Chip

March 19, 2013

Re: [10/10] winemenubuilder: Put appbundle icon inside the bundle.

by Ken Thomases

On Mar 17, 2013, at 12:18 PM, Per Johansson wrote: > + CFStringRef iconstr = CFStringCreateWithCString(NULL, icon, CFStringGetSystemEncoding()); Another place to use CFStringCreateWithFileSystemRepresentation(). -Ken

March 19, 2013

Re: [7/10] winemenubuilder: Create a basic Info.plist.

by Ken Thomases

On Mar 17, 2013, at 12:18 PM, Per Johansson wrote: > + namestr = CFStringCreateWithCString(NULL, link_name, CFStringGetSystemEncoding()); You should use CFStringCreateWithFileSystemRepresentation() to create CFStrings from POSIX path strings. > + pathstr = CFStringCreateWithCString(NULL, path, CFStringGetSystemEncoding()); Same thing here. > + data = CFPropertyListCreateData(NULL, propertyList, format, 0, &err); Since you don't use "err", just pass NULL. -Ken

March 19, 2013

Parameterizing the places Wine suggests people report bugs

by Josh DuBois

Hi all, There are a couple of places wine suggests that people report bugs to wine-devel. The crash reporting dialog in winedbg suggests this, and a couple of places in code refer to wine-devel(a)winehq.org in error messages. The configure script has a PACKAGE_BUGREPORT macro which could be used to allow someone building wine to redirect those errors somewhere else. At least for wine's crash dialog, an alternate approach for a repackager to redirect bug reports would be to read the url / email address for where to send bugs from the registry. Would Wine accept a patch for making winedbg read the 'destination' for bug reports from the registry or some other runtime variable? Or would the project prefer to have all the bug reports routed directly to winehq, so that a repackager who wants to re-route bugs would have to build that facility on its own?

March 19, 2013

Re: msvcr110: add a stub for __crtGetShowWindowMode

by Dmitry Timoshkov

Austin English <austinenglish(a)gmail.com> wrote: > +int CDECL __crtGetShowWindowMode(void) > +{ > + FIXME(": stub!\n"); > + return 0; > +} Isn't it supposed to return SW_SHOW and other SW_xxx commands, specified at process creation time (SW_SHOWDEFAULT)? In that case returning 0 (SW_HIDE) is not a very good choice. -- Dmitry.

March 18, 2013

Re: [0/10] Launcher app bundles on OS X

by Per Johansson

On Mon, Mar 18, 2013 at 9:13 PM, C.W. Betts <computers57(a)hotmail.com> wrote: > A quick glance at the source revealed that you removed static from some functions. Why? If a function isn't being called outside a source file, it's better to have it be static. This includes functions referenced in structs (but not the structs themselves). Hi, I believe I only removed static where it was needed. Utility functions are kept in winemenubuilder.c, and some of them are referenced from xdg.c and/or appbundler.c. Thus static was removed from those. Regards, -- Per Johansson

March 18, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by Ken Thomases

On Mar 18, 2013, at 4:04 PM, Charles Davis wrote: > On Mar 18, 2013, at 2:23 PM, Ken Thomases wrote: > >> On Mar 18, 2013, at 2:15 PM, Ken Thomases wrote: >> >>> On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote: >>> >>>> On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >>>> >>>>> Doesn't support right-clicks, mouse moves, or notification balloons. >>>> Notification balloons can probably be done using either Notification Center or Growl >>> >>> Yeah, I was thinking that the notification center is the right way to go. >> >> Josh reminds me that only code-signed apps can use the Notification Center, so that may not be very useful. *sigh* > So what? [...] > > Though I wonder if it's a good idea to ask users to create a self-signed certificate just so Wine can use the Notification Center... You answered your own question. ;) > At this point, though, I'm wondering if it wouldn't just be easier to have Explorer draw the balloons itself à la Windows XP. Probably. I also considered using an NSPopover anchored to the status item. Again, that's 10.7+ functionality. -Ken

March 18, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by Charles Davis

On Mar 18, 2013, at 2:23 PM, Ken Thomases wrote: > On Mar 18, 2013, at 2:15 PM, Ken Thomases wrote: > >> On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote: >> >>> On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >>> >>>> Doesn't support right-clicks, mouse moves, or notification balloons. >>> Notification balloons can probably be done using either Notification Center or Growl >> >> >> Yeah, I was thinking that the notification center is the right way to go. > > Josh reminds me that only code-signed apps can use the Notification Center, so that may not be very useful. *sigh* So what? It's actually possible to self-sign executables. (If you don't have a real code-signing certificate, you have to do this to build and use GDB or LLDB from source on Mac OS.) You probably won't be able to distribute the resulting executable, though. LLDB has a nice document describing how you'd go about creating a self-signed certificate. You can find it here: http://llvm.org/viewvc/llvm-project/lldb/trunk/docs/code-signing.txt Though I wonder if it's a good idea to ask users to create a self-signed certificate just so Wine can use the Notification Center... >> I'm not sure if we want to use Growl given that it's not part of the OS, but I guess I don't feel very strongly one way or another. Most apps that want Growl come with their own Growl framework. Obviously, we can't do that for vanilla Wine. What we really need for this is some way to dynamically load and use frameworks (according to the standard rules for finding frameworks). That way, we can check for Growl at runtime and use it if it's present. I have a small library that wraps the CFBundle API in a dlopen(3)-style API. (As a matter of fact, I wrote it specifically to see if we could do something like that for Wine. As I recall, it was around the time we got OpenAL support.) Or, we could just check for it in configure (like C.W. said), if that's easier. I guess it really depends on AJ, and we won't be hearing from him for a while. At this point, though, I'm wondering if it wouldn't just be easier to have Explorer draw the balloons itself à la Windows XP. Chip

March 18, 2013

Mac driver status: ready for contributions

by Ken Thomases

Hi, With my latest set of patches, the Mac driver in Wine is up to speed with what's in CrossOver 12 (plus some extra goodies). Of course, that's not to say that the Mac driver is done, but if you were holding off getting involved because you figured big chunks of already-done work were pending, that's no longer the case. From here on out, all work on the Mac driver will be new coding, so it's ready for anybody to contribute. Testing is one excellent way of contributing. I encourage any Mac Wine users to enable the Mac driver, test your favorite programs, and file bugs for any new problems (and there _will_ be bugs to find). Please try to distinguish between general Wine bugs which also happen with the X11 driver vs. bugs which only happen with the Mac driver. Currently, you have to set a registry key to enable the Mac driver: [HKEY_CURRENT_USER\Software\Wine\Drivers] "Graphics"="mac,x11" Swap the order to "x11,mac" – or just remove the "Graphics" value altogether – to revert to using the X11 driver. Aside from bugs, the Mac driver is pretty close to complete. Here's a list of stuff that I know is incomplete. Not all of these things are hard. In fact, I expect some will be quite easy and the only reason I didn't get to them was to get broader coverage of general areas of functionality by CrossOver 12's release. * The Quit menu item should do a graceful shutdown of the Windows process rather than abruptly terminating it. Same for requests to quit from the system such as during logout, restart, or shutdown. * A registry setting to govern whether full-screen at full-resolution should capture the display (which will disable active screen corners but also application switching and UI overlays). Full-screen at other resolutions already captures the displays. * A registry setting to govern whether the Option/Alt key sends an Alt keystroke instead of accessing additional characters from the Mac keyboard layout. * Support for maximizing windows. * Reduce or eliminate flashes of black before windows (re)draw. * Support for Mac input methods (Asian languages, press-and-hold, keyboard and character viewers). * Rework the Graphics tab of winecfg. Most of it doesn't apply to the Mac driver. As Mac-driver-specific registry settings are implemented, they may need to be revealed in winecfg. * Maybe implement a Cocoa Preferences dialog for the Mac driver's settings as an alternative to putting them in winecfg. * Activating a Wine GUI process all of whose windows are minimized should maybe unminimize one. * Add some additional standard items to the Mac menus, such as Hide Wine, Hide Others, and Show All. * Support for metafile clipboard formats. * Support for mouse capture. * Improved system tray functionality like detecting mouse moves and right-clicks and presenting notification balloons. * Support localization of the few UI strings in the Mac driver (basically the menu item titles). (Charles Betts has already submitted work to support disabling and reenabling the screen saver. As I understand it, Alexandre has some unspecified ideas about how he wants to change the driver interface for the SystemParametersInfo entry point.) Cheers, Ken

March 18, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by C.W. Betts

On Mar 18, 2013, at 2:23 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > On Mar 18, 2013, at 3:15 PM, Ken Thomases wrote: > >> On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote: >> >>> On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >>> >>>> Doesn't support right-clicks, mouse moves, or notification balloons. >>> Notification balloons can probably be done using either Notification Center or Growl >> >> Yeah, I was thinking that the notification center is the right way to go. > > Josh reminds me that only code-signed apps can use the Notification Center, so that may not be very useful. *sigh* > I can see that not being an issue on CrossOver, but yes, I doubt that the average person compiling Wine on their Macs has a proper certificate to sign it. As for implementing Growl, a configure check can be done, and a warning shown if a) they are compiling the Mac driver and b) they do not have the Growl development framework installed. This would, however, probably require some work to maintain the Growl framework's presence so that, in example, Wine compiled by MacPorts can be moved to another computer that does not have the Growl framework installed in either /Library/Frameworks or ~/Library/Frameworks. I have done some work with the OS X linker in my project PlayerPRO to get a command-line app to access the framework that is in the same directory of the program, but other than that my linker skills aren't that great.

March 18, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by Ken Thomases

On Mar 18, 2013, at 3:15 PM, Ken Thomases wrote: > On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote: > >> On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: >> >>> Doesn't support right-clicks, mouse moves, or notification balloons. >> Notification balloons can probably be done using either Notification Center or Growl > > Yeah, I was thinking that the notification center is the right way to go. Josh reminds me that only code-signed apps can use the Notification Center, so that may not be very useful. *sigh* -Ken

March 18, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by Ken Thomases

On Mar 18, 2013, at 3:03 PM, C.W. Betts wrote: > On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > >> Doesn't support right-clicks, mouse moves, or notification balloons. > Notification balloons can probably be done using either Notification Center or Growl Yeah, I was thinking that the notification center is the right way to go. Of course, we need to code support for it in a way that still builds and runs on 10.6. I'm not sure if we want to use Growl given that it's not part of the OS, but I guess I don't feel very strongly one way or another. -Ken

March 18, 2013

Re: [0/10] Launcher app bundles on OS X

by C.W. Betts

A quick glance at the source revealed that you removed static from some functions. Why? If a function isn't being called outside a source file, it's better to have it be static. This includes functions referenced in structs (but not the structs themselves). A more seasoned developer is welcome to correct me if I'm wrong. On Mar 17, 2013, at 11:18 AM, Per Johansson <per(a)morth.org> wrote: > I'm resending this now after testing it with the Mac driver. > They work well together, the launcher bundle will first briefly > appear in the dock, then exit, and the mac driver icon appears. > > It's split it up into parts for easier review. I've also tried > to make sure that even if e.g. part 6 fails review, part 1 - 5 > can still be applied. > > File type associations are not included because I plan to move > that code into the mac driver but haven't gotten around to it yet. > > Regards, > -- > Per Johansson > > >

March 18, 2013

Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.

by C.W. Betts

On Mar 17, 2013, at 9:40 PM, Ken Thomases <ken(a)codeweavers.com> wrote: > > Doesn't support right-clicks, mouse moves, or notification balloons. Notification balloons can probably be done using either Notification Center or Growl, but either due to an out-of-date version of Growl in Twitter or a bug in Growl itself, clicking on the Growl notification doesn't bring Twitter to the front for me. > --- > dlls/winemac.drv/Makefile.in | 2 + > dlls/winemac.drv/cocoa_status_item.m | 175 +++++++++++++++++ > dlls/winemac.drv/event.c | 5 + > dlls/winemac.drv/image.c | 2 +- > dlls/winemac.drv/macdrv.h | 3 + > dlls/winemac.drv/macdrv_cocoa.h | 13 ++ > dlls/winemac.drv/systray.c | 358 ++++++++++++++++++++++++++++++++++ > dlls/winemac.drv/winemac.drv.spec | 3 + > 8 files changed, 560 insertions(+), 1 deletions(-) > create mode 100644 dlls/winemac.drv/cocoa_status_item.m > create mode 100644 dlls/winemac.drv/systray.c > > <0004-winemac-Implement-rudimentary-support-for-system-tra.patch>

March 18, 2013

Re: [website] Portuguese translation for release 1.5.25

by Lucas Zawacki

Good catch. I'm re-sending it.

March 18, 2013

Re: [website] Portuguese translation for release 1.5.25

by Bruno Jesus

2013/3/18 Lucas Zawacki <lfzawacki(a)gmail.com>: > From: Lucas Zawacki <lfzawacki(a)gmail.com> > > --- > news/pt/2013030101.xml | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > create mode 100644 news/pt/2013030101.xml > > diff --git a/news/pt/2013030101.xml b/news/pt/2013030101.xml > new file mode 100644 > index 0000000..ae13cfd > --- /dev/null > +++ b/news/pt/2013030101.xml > @@ -0,0 +1,14 @@ > +<news> > +<date>1 de Março de 2013</date> > +<title>Wine 1.5.25 lançado</title> > +<body> > + A versão de desenvolvimento 1.5.25 já está disponível agora. > + <a href="{$root}/announce/1.5.25">Novidades</a> desta versão: > +<ul> > + <li>Suporte adequado ao cursor no driver de Mac.</li> > + <li>Correções no suporte a escrita direita-para-esquerda no RichEdit.</li> > + <li>Versão inicial da uma fonte Wingdings.</li> Should be: Versão inicial de uma fonte Wingdings. Or: Versão inicial da fonte Wingdings. > + <li>Várias correções de bugs.</li> > +O código fonte está <a href="http://prdownloads.sourceforge.net/wine/wine-1.5.25.tar.bz2">disponível agora</a>. > +Pacotes binários estão em processo de construção e irão aparecer em breve em suas respectivas <a href="{$root}/download">sessões da área de downloads</a>. > +</body></news> > -- > 1.7.9.5 > > > Best wishes, Bruno

March 18, 2013

Re: usp10: Support GPOS extension lookup subtables

by Aric Stewart

Hi Phil, Thank you for being patient I am back from vacation and reviewing your patch. The logic looks great and just fine. Do you happen to have a font that uses lookup type 9 that I can run my usp test suite on to make sure it functions as expected? thanks! -aric On 3/8/13 10:47 AM, Phil Krylov wrote: > > > >

March 18, 2013

Re: WineD3D performance

by Graham Knap

Stefan Dösinger wrote: > Note however that improving performance is not a weekend's job. > It needs a lot of careful profiling, testing and bugfixing. Of course. I'm also starting with no knowledge of D3D/OpenGL, so that's going to slow me down rather a lot. > Depending on your settings, you may be CPU or GPU limited. As noted in Wine bug 24558, I find that need to use taskset to get any useful amount of performance. $ taskset -a -p 2 `pidof SC2.exe` $ taskset -p 1 `pidof SC2.exe` I am not sure that this is a Wine bug at all. I think it is merely a difference in the OS's scheduler and power management. SC2 probably should set thread CPU affinity, but I don't think it does that. The graphic detail and display resolution settings in the game don't make a huge difference to the frame rate, so I think we're CPU-bound. > In the CPU limited case, I believe the main reason is that native is able > to move a lot of the CPU-side work to a different thread, thus utilizing > two CPU cores. You can test that theory by forcing Windows to run with > one CPU only (use msconfig for that) and comparing performance again. That makes a difference for sure. Frame rate during a small recorded battle: * Linux + Wine: about 18 FPS * Windows XP (native D3D) with 1 core: about 48 FPS * Windows XP (native D3D) with 2 cores: about 67 FPS We have spent some time with strace, latencytop, and WINEDEBUG, looking for ways to reduce latency and scheduler thrash. I don't think there is much room for optimization here. However, I would still like to know what is calling various functions that cause latency, such as Sleep() or anything that requires a server call. Does anyone know how I can get that information? (I just want to know whether the call is coming from SC2, or from somewhere within Wine.) We have tried using Linux's "perf" tool to find out what code paths should be given attention first. However, even though I have compiled Wine with "-ggdb", perf seems unable to show symbols for some parts of Wine, notably ntdll and kernel32, but probably also others. Does anyone know why? Is there a workaround, or a better method altogether? The "Performance" page on the Wine Wiki suggests that it may be possible to use Visual Studio's profiling tools. Can anyone offer any tips for compiling and profiling WineD3D with Visual Studio? -- graham

March 18, 2013

Re: wined3d: add NVidia GeForce 315M

by Daniel Jeliński

2013/3/18, Stefan Dösinger <stefandoesinger(a)gmail.com>: > *) The upcoming GLX_MESA_query_renderer extension will provide a good way to > query this info. If this extension will be adopted by the binary drivers is > an open question. > > *) The X.org log would be an extremely ugly way to read this info. The log > is at an unknown place, might be readable only by root or might not exist at > all. There format of the log messages might change. > > *) There are vendor specific X11, GLX and GL extensions to query some GPU > info > > *) Wined3d should not deal with GLX_MESA_query_renderer or any other X11 > specific things. This should be handled by winex11.drv. Thanks for indulging my curiosity. I thought that if Xorg.log contains the card name, it implies that X server knows the name and can share it. Seems that I was wrong. >> {"GT 320M", CARD_NVIDIA_GEFORCE_GT320M}, /* Geforce 300 >> - midend mobile */ >> + {"315M", CARD_NVIDIA_GEFORCE_315M}, /* Geforce >> 300 - midend mobile */ >> {"320M", CARD_NVIDIA_GEFORCE_320M}, /* Geforce >> 300 - midend mobile */ > Afaics this table is sorted in descending order, for consistency you should > swap the 315M and 320M The order here is quite weird, but now that you mention it, cards with the same comment actually are sorted. I'll send an updated patch.

March 18, 2013

Re: msxml3: Embed user/password to uri used to create a moniker (try2)

by Nikolay Sivov

On 3/15/2013 21:43, Nikolay Sivov wrote: > try2: fixed builder leak > Please ignore this one, it removes too much, I have a better version.

March 18, 2013

Re: wined3d: add NVidia GeForce 315M

by Stefan Dösinger

Am 17.03.2013 um 17:08 schrieb Daniel Jeliński <djelinski1(a)gmail.com>: > why can't we get these values from X? They are definitely present in > my Xorg.log. Propose a patch. A few notes though: *) The upcoming GLX_MESA_query_renderer extension will provide a good way to query this info. If this extension will be adopted by the binary drivers is an open question. *) The X.org log would be an extremely ugly way to read this info. The log is at an unknown place, might be readable only by root or might not exist at all. There format of the log messages might change. *) There are vendor specific X11, GLX and GL extensions to query some GPU info *) Wined3d should not deal with GLX_MESA_query_renderer or any other X11 specific things. This should be handled by winex11.drv. > {"GT 320M", CARD_NVIDIA_GEFORCE_GT320M}, /* Geforce 300 - midend mobile */ > + {"315M", CARD_NVIDIA_GEFORCE_315M}, /* Geforce 300 - midend mobile */ > {"320M", CARD_NVIDIA_GEFORCE_320M}, /* Geforce 300 - midend mobile */ Afaics this table is sorted in descending order, for consistency you should swap the 315M and 320M

March 17, 2013

Re: Updating WineWiki Theme?

by Francois Gouget

On Fri, 15 Mar 2013, André Hentschel wrote: [...] > Please open bugs for them so they can be fixed later. Done: * Bug 33230 - The Wiki produces ugly tables http://bugs.winehq.org/show_bug.cgi?id=33230 * Bug 33231 - Wiki: Trouble sending notification emails http://bugs.winehq.org/show_bug.cgi?id=33231 -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ 145 = 1! + 4! + 5!

March 16, 2013

Re: Updating WineWiki Theme?

by Kyle Auble

On Friday, March 15, 2013 7:45 PM, André Hentschel wrote: > What i'd like is that you first upgrade the theme for now, whatever changes it may need. > For this task don't care about search fields, tables, sponsoring, ... > The result should be a wine wiki with the same quality and/or bugs as we have now, just with a new look. > After that is pushed and running and you fixed remaining bugs with the new look, you can then start fixing other bugs, > changes things (keep in mind to talk to Jeremy Newman) and upgrade the wiki software itself. Sounds like a good plan. I've been testing the new theme with a v1.9 install of Moinmoin, but I should be able to create a branch and backport that to v1.5 pretty easily. After that, I can work on the tweaks and we can look into upgrading the underlying software. Dimi mentioned that he has some maintenance scripts written up, and I also have some on my Bitbucket account so getting the tarball up again and converting the data shouldn't take too long. - Kyle

March 15, 2013

Vacation

by Alexandre Julliard

Folks, There won't be any commits next week, as I'll be out skiing... -- Alexandre Julliard julliard(a)winehq.org

March 15, 2013

Re: [1/3] Make mac driver the default on OS X (try 2)

by Alexandre Julliard

Josh DuBois <duboisj(a)codeweavers.com> writes: > Ken also noted that the error reporting could be simpler if it were in > gdi instead of user32. My impression, after having poked at it a > while with Ken, is that it is in theory possible that an application > which never tried to create a window would print less noise with the > errors being reported as they are here than it would if the errors > were reported directly from gdi, so that this may be superior because > it is quieter. However, I'm not an expert and if the rigamarole is > not worth the extra lines, I'm sure Alexandre will indicate so by not > applying this. It needs to be printed on window creation because of command line apps, but that's not a reason for doing something as ugly and complicated as this... -- Alexandre Julliard julliard(a)winehq.org

March 15, 2013

Re: d3dx9 [patch 1/2, try 3]: Implement D3DXSHEvalSphericalLight

by Matteo Bruni

2013/3/15 Nozomi Kodama <nozomi.kodama(a)yahoo.com>: > Patch series is fine to me.

March 15, 2013

[1/3] Make mac driver the default on OS X

by Joerg-Cyril.Hoehle＠t-systems.com

I wrote: >The [notepad] menu bar sometimes was refreshed badly, causing some texts therein >to be displayed twice, shifted by a few pixels, or with varying fonts. That looks much like bug #33154 http://bugs.winehq.org/show_bug.cgi?id=33154 filed about Linux/X11. Nothing specific to winemac -- at least the "shifted by a few pixels" part, I'll have to check again about the "varying fonts" issue (boldface might result from shifting by one pixel). I believe Daniel Jelinski expressed the issue well in #32334: http://bugs.winehq.org/show_bug.cgi?id=32334#c19 >it seems that it also made calculating text extents inconsistent with drawing text. Somehow it feels strange that Wine's built-in notepad application would depend on a metrics-compatible Tahoma font. Compatible or not, Wine should probably compute sizes aka. extents consistently. Regards, Jörg Höhle

March 15, 2013

Re: [PATCH 1/6] atl100/tests: Added class name test for AtlAxWinInit.

by Qian Hong

On Fri, Mar 15, 2013 at 9:24 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: > It's not useful to load this dynamically if you are not going to check > that it was found. Thanks very much, I'll send an improved version. -- Regards, Qian Hong - http://www.codeweavers.com

March 15, 2013

winmm: Fix notification when recording using MSACM.

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, Alexandre Julliard wrote: >> To achieve this, we eliminate the tail call and turn it into a loop, >> which allows us to preserve the local state in the variable "last". >Please do this as a proper loop, not with a goto. In bug #33155 I explained that Wine's PullACMData needs a redesign http://bugs.winehq.org/show_bug.cgi?id=33155 The core issue is the fundamental impedance mismatch between mmdevapi capture and WinMM/ACM recording w.r.t. packet/headers sizes that shows off depending on codec's block size. I have no plans to fix that. Any takers? As a consequence, I won't spend time now refactoring the existing misdesigned code into a set of nested loops -- still as misdesigned -- just to eliminate a goto. So the choice was: - Perform a tiny surgical change now, using a goto, with the benefit that 1:1 ACM codecs like µ-Law or A-Law would start working, while all others are left in the cold, waiting for a redesign. - Leave as is, waiting for somebody to perform the redesign. Meanwhile, ACM recording doesn't work at all and the whole function could as well be replaced by a single FIXME line. Regards, Jörg Höhle

March 15, 2013

Re: [PATCH 1/6] atl100/tests: Added class name test for AtlAxWinInit.

by Alexandre Julliard

Qian Hong <qhong(a)codeweavers.com> writes: > @@ -562,6 +562,34 @@ static void test_source_iface(void) > ok(maj_ver == 4 && min_ver == 0, "ver = %d.%d\n", maj_ver, min_ver); > } > > +static void test_ax_win(void) > +{ > + BOOL ret; > + WNDCLASSEXW wcex; > + static const WCHAR AtlAxWin100[] = {'A','t','l','A','x','W','i','n','1','0','0',0}; > + static const WCHAR AtlAxWinLic100[] = {'A','t','l','A','x','W','i','n','L','i','c','1','0','0',0}; > + static HMODULE huser32 = 0, hinstance = 0; > + static BOOL (WINAPI *pGetClassInfoExW)(HINSTANCE, LPCWSTR, WNDCLASSEXW *); > + > + huser32 = GetModuleHandleA("user32.dll"); > + pGetClassInfoExW= (void *)GetProcAddress(huser32, "GetClassInfoExW"); It's not useful to load this dynamically if you are not going to check that it was found. -- Alexandre Julliard julliard(a)winehq.org

March 15, 2013

Re: mmdevapi: Prevent 64 bit overflow within a few days of audio device use. (try 3)

by Alexandre Julliard

<Joerg-Cyril.Hoehle(a)t-systems.com> writes: > The only comments about v2 were Michael Stefaniuc > suggesting a refactoring and me explaining why > in this particular case, that would not be a good idea. > http://www.winehq.org/pipermail/wine-devel/2013-March/099068.html It's still a good idea, even if the function is ultimately used only in one place. > For v3, I've renamed MulDiv64 to UMulDiv64. Unlike QueryPerformanceCounter, > mmdevapi GetPosition operates on unsigned values. That's a bad name. Don't make internal functions look like Win32 APIs. -- Alexandre Julliard julliard(a)winehq.org

March 15, 2013

Re: Updating WineWiki Theme?

by André Hentschel

Am 15.03.2013 04:17, schrieb Francois Gouget: > * A default table is totally ugly: it has no border and no cell padding > so it does not look like a table at all, just text appearing at random > on the page. Here is an example: > http://wiki.winehq.org/FrancoisGouget > > * During the preview the tables look nothing like after the preview. In > fact they look good enough during the preview, it's only on the final > page that they get ugly. This makes the preview totally useless as far > as tables are concerned :-( ... > Oh, and the Wiki seems to have trouble sending emails(*) so saving > changes is super slow. But that's probably not something you can do > anything about. Please open bugs for them so they can be fixed later. Am 14.03.2013 20:58, schrieb Kyle Auble: > I did want to ask for input on a few things: > 1. Should we just keep Moinmoin's search, replace it with the custom > Google search, or offer both? For now keep it as it is. > 2. How about the icon? I can get the standard WineHQ one to display > fine, but I'm not sure about which approach to take. Essentially, do > what's simplest for us, or should I make it play well with other > themes too? The simplest approach sounds good :) > 3. Finally, the credits are currently just text links. In the future > though, if Dimi wanted to, we could have a "sponsored by Lattica" > banner like the Codeweavers ones on the other pages. Should not be a problem... What i'd like is that you first upgrade the theme for now, whatever changes it may need. For this task don't care about search fields, tables, sponsoring, ... The result should be a wine wiki with the same quality and/or bugs as we have now, just with a new look. After that is pushed and running and you fixed remaining bugs with the new look, you can then start fixing other bugs, changes things (keep in mind to talk to Jeremy Newman) and upgrade the wiki software itself. -- Best Regards, André Hentschel

March 15, 2013

Re: Updating WineWiki Theme?

by Francois Gouget

On Thu, 14 Mar 2013, Kyle Auble wrote: [...] > I did want to ask for input on a few things: [...] Sorry, I don't have anything useful to contribute on these. However since this is about the Wiki theming I'll mention a problem I ran into last week-end: tables. There are two problems with them: * A default table is totally ugly: it has no border and no cell padding so it does not look like a table at all, just text appearing at random on the page. Here is an example: http://wiki.winehq.org/FrancoisGouget * During the preview the tables look nothing like after the preview. In fact they look good enough during the preview, it's only on the final page that they get ugly. This makes the preview totally useless as far as tables are concerned :-( In the page I was working on I worked around this by specifying CSS styles for every single cell. The resulting wiki source code is pretty ugly but at least the result looks like tables. http://wiki.winehq.org/WineTestBotVMs#hw_tables Oh, and the Wiki seems to have trouble sending emails(*) so saving changes is super slow. But that's probably not something you can do anything about. (*) I get: Status of sending notification mails: [en] fracting: Connection to mailserver 'mail.lattica.com' failed: (101, 'Network is unreachable') -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ RFC 2549: ftp://ftp.isi.edu/in-notes/rfc2549.txt IP over Avian Carriers with Quality of Service

March 15, 2013

Re: WineD3D performance

by Stefan Dösinger

Am 14.03.2013 um 20:07 schrieb Graham Knap <graham.knap(a)gmail.com>: > I would like to try to help with improving the performance of > StarCraft II on Wine. I'm working with a friend. Over the past couple > of weeks, we have tried to read whatever documentation we could find. > We have tried a couple of different approaches to profiling, without > much success. That's great :-) Note however that improving performance is not a weekend's job. It needs a lot of careful profiling, testing and bugfixing. > Can anyone offer some hints on how to find the most significant causes > of the performance difference? Any tips for effective profiling or > tracing? Depending on your settings, you may be CPU or GPU limited. A rule of thumb way to find out is to change the game's resolution - if increasing the resolution causes a big framerate drop, you're probably GPU limited. If the framerate stays the same, you're CPU limited. In the GPU limited case, improving the situation requires a tedious work to find shaders that execute slower, finding out why they are slower and then fixing things. It also needs double-checking with other games and GPUs to make sure that you're not optimizing one game at the cost of another. In the CPU limited case, I believe the main reason is that native is able to move a lot of the CPU-side work to a different thread, thus utilizing two CPU cores. You can test that theory by forcing Windows to run with one CPU only (use msconfig for that) and comparing performance again. Aside of the worker thread, there may be some parts of the wined3d code that can be optimized. Like the GPU side work, this requires a lot of careful testing, fixing and double-checking. Stefan

March 14, 2013

Re: Aric Stewart : usp10: Update diacritical script handling for non OpenType Itemization.

by Phil Krylov

Hello, On Fri, Mar 15, 2013 at 12:52 AM, Aric Stewart <aric(a)codeweavers.com> wrote: > I am just getting back from a vacation so I will review this and your other patch early next week when I am fully back. > > Thanks for doing work here! Thanks for replying, wasn't sure you saw my posts. And of course thanks for your work too. -- Ph.

March 14, 2013

Re: Aric Stewart : usp10: Update diacritical script handling for non OpenType Itemization.

by Aric Stewart

On 3/7/13 4:57 AM, Phil Krylov wrote: > Hello Aric, > > On Tue, Feb 5, 2013 at 11:25 PM, Alexandre Julliard <julliard(a)winehq.org> wrote: >> Module: wine >> Branch: master >> Commit: ef4b7c5948e43d4ceeed98ba4ddbf6c222259d36 >> URL: http://source.winehq.org/git/wine.git/?a=commit;h=ef4b7c5948e43d4ceeed98ba4… >> >> Author: Aric Stewart <aric(a)codeweavers.com> >> Date: Tue Feb 5 08:57:44 2013 -0600 >> >> usp10: Update diacritical script handling for non OpenType Itemization. > > (snip) > >> + for (j = i-1; j >= 0 && scripts[j] == first_script && pwcInChars[j] != Numeric_space; j--) >> + { >> + WORD original = scripts[j]; >> + if (original == Script_Ideograph || original == Script_Kana || original == Script_Yi || original == Script_CJK_Han || original == Script_Bopomofo) >> + { >> + asian = TRUE; >> + break; >> + } >> + if (original != Script_MathAlpha && scriptInformation[scripts[j]].props.fComplex) >> + break; >> + scripts[j] = scripts[i]; >> + if (original == Script_Punctuation2) >> + break; >> + } >> + if (scriptInformation[scripts[j]].props.fComplex || asian) >> + scripts[i] = scripts[j]; > > I am not sure about the logic of this, so I can't commit a good patch, > but in the last statement you definitely access scripts[-1] when j > reaches the value of 0. Which may cause a crash here or (in my case) > later. > > -- Ph. > > Hi there, I am just getting back from a vacation so I will review this and your other patch early next week when I am fully back. Thanks for doing work here! -aric

March 14, 2013

Updating WineWiki Theme?

by Kyle Auble

Hi everybody, Although my hiatus was longer than expected, I've been making progress with updating the wiki code over the past few days (spring break is a very good thing). My workflow is a bit non-linear so I want to sort my changes into patches first, but fingers-crossed, I'll be pushing them to my Bitbucket account by the weekend. I did want to ask for input on a few things: 1. Should we just keep Moinmoin's search, replace it with the custom Google search, or offer both? 2. How about the icon? I can get the standard WineHQ one to display fine, but I'm not sure about which approach to take. Essentially, do what's simplest for us, or should I make it play well with other themes too? 3. Finally, the credits are currently just text links. In the future though, if Dimi wanted to, we could have a "sponsored by Lattica" banner like the Codeweavers ones on the other pages. After I get the theme together & test it some more on different browsers, I can look into migrating the data to the newest Moinmoin version. -Kyle

March 14, 2013

WineD3D performance

by Graham Knap

Hi everyone I would like to try to help with improving the performance of StarCraft II on Wine. I'm working with a friend. Over the past couple of weeks, we have tried to read whatever documentation we could find. We have tried a couple of different approaches to profiling, without much success. My friend got WineD3D running on Windows. We built Wine from current source code, tested this on two different configurations, and got similar results. - my Core 2 E8500 + GeForce 9600GT + Windows XP x86 - his i5-2500 + GeForce 660 + Windows 7 x64 It looks like WineD3D is the cause of most of the performance difference. Windows with its own D3D gave us at least twice the frame rate of Windows + WineD3D. There is some difference between the numbers for Windows + WineD3D and Linux + Wine, but it's much smaller, i.e. something like 25%. Can anyone offer some hints on how to find the most significant causes of the performance difference? Any tips for effective profiling or tracing? Thanks. -- graham

March 14, 2013

Re: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight

by Matteo Bruni

2013/3/14 Nozomi Kodama <nozomi.kodama(a)yahoo.com>: > No news about these patches? > > > Nozomi? > > Hmm, I was under the impression that these patches were already picked up in git, but I see I was mistaken. Anyway they look okay to me, except for a bit in the tests from the first patch that I noticed just now by reading the patches again: + else if (j >= order * order) + expected = test[l].red_received[j]; That somewhat defies the test, since you are then comparing a value with itself in the ok(). You can just use the original value (i.e. "1.01f + j" for the red component) instead. Same issue with green and blue of course. Cheers, Matteo. > ________________________________ > De : Nozomi Kodama <nozomi.kodama(a)yahoo.com> > À : "wine-devel(a)winehq.org" <wine-devel(a)winehq.org> > Envoyé le : Lundi 11 mars 2013 17h11 > Objet : RE: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight > > Hello > > Any problem with the patch > http://source.winehq.org/patches/data/94777 > > and > http://source.winehq.org/patches/data/94776 ? > > Nozomi. > >

March 14, 2013

Re: winegcc: Add support for -Ttext-segment linker flag to set image base of a dll. (try 2)

by Alexandre Julliard

Tijl Coosemans <tijl(a)coosemans.org> writes: > Loading a library at a specific address not only depends on the > compile-time linker, but also on the run-time loader, so it's > probably best to implement -image-base only on PLATFORM_* where > the loader supports it. Since a target platform would then have > to choose to use PLATFORM_ELF explicitly I think it's reasonable > for PLATFORM_ELF to have a dependency on binutils>=2.20 (or > compatible) rather than a run-time test for -Ttext-segment > support. The gold linker doesn't support it even in recent binutils versions, and some systems use that by default. -- Alexandre Julliard julliard(a)winehq.org

March 14, 2013

Re: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight

by Nozomi Kodama

No news about these patches? Nozomi? ________________________________ De : Nozomi Kodama <nozomi.kodama(a)yahoo.com> À : "wine-devel(a)winehq.org" <wine-devel(a)winehq.org> Envoyé le : Lundi 11 mars 2013 17h11 Objet : RE: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight Hello Any problem with the patch http://source.winehq.org/patches/data/94777 and http://source.winehq.org/patches/data/94776 ? Nozomi.

March 14, 2013

Re: winegcc: Add support for -Ttext-segment linker flag to set image base of a dll. (try 2)

by Tijl Coosemans

On 2013-03-06 12:25, Alexandre Julliard wrote: > Alexandre Julliard <julliard(a)winehq.org> writes: >> Tijl Coosemans <tijl(a)coosemans.org> writes: >>> On 03-03-2013 17:44, André Hentschel wrote: >>>> You would rather call it PLATFORM_FREEBSD >>> >>> Well, the flag isn't FreeBSD specific. It works on all ELF targets >>> (including Linux which then wouldn't need prelink). >> >> Then it should be used by default, and you don't need to add a new >> platform for it. > > Actually, I looked into this and it isn't widely supported at all, so it > can't be the default. Ideally it would need a run-time check to see if > the option works. Wouldn't a run-time check be too much of a performance hit? Loading a library at a specific address not only depends on the compile-time linker, but also on the run-time loader, so it's probably best to implement -image-base only on PLATFORM_* where the loader supports it. Since a target platform would then have to choose to use PLATFORM_ELF explicitly I think it's reasonable for PLATFORM_ELF to have a dependency on binutils>=2.20 (or compatible) rather than a run-time test for -Ttext-segment support.

March 13, 2013

Re: [PATCH 2/2] ole32: Don't pass CLSCTX_INPROC_SERVER and CLSCTX_INPROC_HANDLER flags to CoCreateInstance in DefaultHandler

by Marvin

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=24750 Your paranoid android. === WINEBUILD (build) === Patch failed to apply

March 13, 2013

Re: gdiplus: Ignore gdiplus page and world transformations when creating a gdi32 clipping region.

by Vincent Povirk

I guess this makes sense, because having the clipping region change with transforms would be weird when drawing recursively and changing the transform. I had assumed clipping regions were in world coordinates because most things are, but I didn't really have any information either way.

March 13, 2013

Re: [PATCH 1/2] wined3d: Put local constants into the shader code again

by Stefan Dösinger

Henri Verbeet <hverbeet(a)gmail.com> schrieb: >This is probably ok, although one concern is applications setting >constants to e.g. NaN or infinity. It may be worth using >ARB_shader_bit_encoding when available, or perhaps even only doing >this when that extension is available. I'll do some testing on that, please skip the patch for now. -- Diese Nachricht wurde von meinem Mobiltelefon mit Kaiten Mail gesendet.

March 13, 2013

Re: [PATCH 1/2] wined3d: Put local constants into the shader code again

by Henri Verbeet

On 13 March 2013 02:19, Stefan Dösinger <stefan(a)codeweavers.com> wrote: > This provides a small GPU-side performance boost on dx10+ capable Nvidia > cards - about 5% in World of Tanks on my Geforce 9600, and more in a > dedicated test program. > > This is essentially a revert of cd7825c89374fb9dd4c20aef2dbfd258713efe6a, > with proper precision. > This is probably ok, although one concern is applications setting constants to e.g. NaN or infinity. It may be worth using ARB_shader_bit_encoding when available, or perhaps even only doing this when that extension is available.

March 13, 2013

Re: [PATCH 1/2] wined3d: Put local constants into the shader code again

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-03-12 18:19, schrieb Stefan Dösinger: > The test program is available at > https://84.112.174.163/~git/perftest file const_gl/const_gl.cpp. My server is currently turned off (and I should move this to github or somewhere at some point), so I've attached the test program here for reference. Resolution and constant mode are hardcoded into it. Tab/space mixup thanks to Visual Studio. Compile the Linux version with g++ const_gl.cpp -o const_gl -lGL -lglut -lGLEW . A d3d version is attached as well. It does not load the same values when using the global constants, but this doesn't seem to affect performance. I haven't tested ARB shaders, but from the WoT results before and after patch 2, their behavior seems to be the same as GLSL and native d3d. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJRP9WcAAoJEN0/YqbEcdMwdmoP/2LvEFpPpmTiZDzxqhp38WfU DKs2S8m1kP86fL55GwLtth289WC9MsQyOosOVOqt8r3YWZyckeisZyhdP2ez2KRV 9kgxMnEXwzcZBtmm/+nrl0FwJghy86AnZ0YJ9PpnCpY0k4VTQYjaWQALlyqtF+qm Uup4UE+m4yR4DoNL/u/161Zxh+OLOA8PXEhWWfDozNL/8yMlRfPs6gzLMN/oKTIL UMA90gesZWPt7/J2Bp0+OeZvMWqNa54A2akxsLvtxVP/rB8baym4iYLn5Y2IPXa2 lBqALc+POMcVk45FCgkzjUw5b/7ck9V5n0PKOw/9Jwrm8/ChHE4i0ZwGEmHzh3So y1d/KydwesYtyCadmlklgJy1J+CxyCd/fNVUmsNtpzDEo6OF7oEagvwBUlTnCdR9 W9B9lmpsDCuBIxEnAy7C3HEZ18o70HdoUsKe5hX8fVn5lnwFPWdp7hBA0nVbO0y0 Fybps006cQmc0Rm9nHJjeJ9JoGzl/iuSX0AnQ5fiJS6fS5RrrGAnFBEb1g2O879X pNSIgBdKUSSfGBoUZKKIy+GZzC+VhDm4UqhtSaYudlUB9pcoMDFYRGQ3Rk48kRuD tJ8Ah2VBYeFUvI+x4Xty9AtOwwFWYejNG+6cNNF3EOALz2JvhFKDEJCI0p7bdmgR XCfPxQyYN7+JGFw8JiO5 =RgiG -----END PGP SIGNATURE-----

March 13, 2013

Re: [PATCH v3 6/7] NFSv4: Add O_DENY* open flags support

by Jeff Layton

On Mon, 11 Mar 2013 14:54:34 -0400 Jeff Layton <jlayton(a)redhat.com> wrote: > On Thu, 28 Feb 2013 19:25:32 +0400 > Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > > > by passing these flags to NFSv4 open request. > > > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > > --- > > fs/nfs/nfs4xdr.c | 24 ++++++++++++++++++++---- > > 1 file changed, 20 insertions(+), 4 deletions(-) > > > > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c > > index 26b1439..58ddc74 100644 > > --- a/fs/nfs/nfs4xdr.c > > +++ b/fs/nfs/nfs4xdr.c > > @@ -1325,7 +1325,8 @@ static void encode_lookup(struct xdr_stream *xdr, const struct qstr *name, struc > > encode_string(xdr, name->len, name->name); > > } > > > > -static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode) > > +static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode, > > + int open_flags) > > { > > __be32 *p; > > > > @@ -1343,7 +1344,22 @@ static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode) > > default: > > *p++ = cpu_to_be32(0); > > } > > - *p = cpu_to_be32(0); /* for linux, share_deny = 0 always */ > > + if (open_flags & O_DENYMAND) { > > > As Bruce mentioned, I think a mount option to enable this on a per-fs > basis would be a better approach than this new O_DENYMAND flag. > > > > + switch (open_flags & (O_DENYREAD|O_DENYWRITE)) { > > + case O_DENYREAD: > > + *p = cpu_to_be32(NFS4_SHARE_DENY_READ); > > + break; > > + case O_DENYWRITE: > > + *p = cpu_to_be32(NFS4_SHARE_DENY_WRITE); > > + break; > > + case O_DENYREAD|O_DENYWRITE: > > + *p = cpu_to_be32(NFS4_SHARE_DENY_BOTH); > > + break; > > + default: > > + *p = cpu_to_be32(0); > > + } > > + } else > > + *p = cpu_to_be32(0); > > } > > > > static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_openargs *arg) > > @@ -1354,7 +1370,7 @@ static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_opena > > * owner 4 = 32 > > */ > > encode_nfs4_seqid(xdr, arg->seqid); > > - encode_share_access(xdr, arg->fmode); > > + encode_share_access(xdr, arg->fmode, arg->open_flags); > > p = reserve_space(xdr, 36); > > p = xdr_encode_hyper(p, arg->clientid); > > *p++ = cpu_to_be32(24); > > @@ -1491,7 +1507,7 @@ static void encode_open_downgrade(struct xdr_stream *xdr, const struct nfs_close > > encode_op_hdr(xdr, OP_OPEN_DOWNGRADE, decode_open_downgrade_maxsz, hdr); > > encode_nfs4_stateid(xdr, arg->stateid); > > encode_nfs4_seqid(xdr, arg->seqid); > > - encode_share_access(xdr, arg->fmode); > > + encode_share_access(xdr, arg->fmode, 0); > > } > > > > static void > > > Other than that, this seems reasonable. > > Acked-by: Jeff Layton <jlayton(a)redhat.com> Oh duh... Please ignore my comment on patch #7 to add a patch for the NFS client. This one does that. That said, there may be a potential problem here that you need to consider. In the case of a local filesystem you'll want to set deny locks using deny_lock_file(). For a network filesystem like CIFS or NFS though, the server will handle that atomically during the open. You need to ensure that you don't go trying to set LOCK_MAND locks on the file once that's done. Perhaps you can use a fstype flag to indicate that the filesystem handles this during the open and doesn't need to try and set a flock lock? -- Jeff Layton <jlayton(a)redhat.com>

March 12, 2013

Re: [PATCH] activeds: Exclude unused headers.

by Alexandre Julliard

Amine Khaldi <amine.khaldi(a)reactos.org> writes: > In this case ending up with windows.h is not needed, so if we: > > #define WIN32_NO_STATUS > #define _INC_WINDOWS > #define COM_NO_WINDOWS_H > > That prevents it. > > Should I send a second patch with these defines ? No, that's ugly, particularly the _INC_WINDOWS one. -- Alexandre Julliard julliard(a)winehq.org

March 12, 2013

Re: [PATCH] activeds: Exclude unused headers.

by Amine Khaldi

On 12/03/2013 11:27, Alexandre Julliard wrote: > Amine Khaldi <amine.khaldi(a)reactos.org> writes: > >> @@ -28,13 +28,10 @@ >> >> #include "windef.h" >> #include "winbase.h" >> -#include "winuser.h" >> >> #include "objbase.h" >> #include "iads.h" >> -#include "adshlp.h" >> >> -#include "wine/unicode.h" >> #include "wine/debug.h" > It's not worth the trouble, particularly since the file ends up > including windows.h anyway. > In this case ending up with windows.h is not needed, so if we: #define WIN32_NO_STATUS #define _INC_WINDOWS #define COM_NO_WINDOWS_H That prevents it. Should I send a second patch with these defines ? Regards, Amine.

March 12, 2013

Re: winmm: Fix notification when recording using MSACM.

by Alexandre Julliard

<Joerg-Cyril.Hoehle(a)t-systems.com> writes: > To achieve this, we eliminate the tail call and turn it into a loop, > which allows us to preserve the local state in the variable "last". Please do this as a proper loop, not with a goto. -- Alexandre Julliard julliard(a)winehq.org

March 12, 2013

Re: [PATCH] activeds: Exclude unused headers.

by Alexandre Julliard

Amine Khaldi <amine.khaldi(a)reactos.org> writes: > @@ -28,13 +28,10 @@ > > #include "windef.h" > #include "winbase.h" > -#include "winuser.h" > > #include "objbase.h" > #include "iads.h" > -#include "adshlp.h" > > -#include "wine/unicode.h" > #include "wine/debug.h" It's not worth the trouble, particularly since the file ends up including windows.h anyway. -- Alexandre Julliard julliard(a)winehq.org

March 12, 2013

RE: d3dx9 [patch 1/2]: Implement D3DXSHEvalSphericalLight

by Nozomi Kodama

Hello Any problem with the patch http://source.winehq.org/patches/data/94777 and http://source.winehq.org/patches/data/94776 ? Nozomi.

March 12, 2013

Re: ntdll: Make msync() asynchronous in NtFlushVirtualMemory (resubmit with real name)

by Mathis Beer

On 03/11/13 21:23, Alexandre Julliard wrote: > Mathis Beer <default_357-line(a)yahoo.de> writes: > >> diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c >> index a99bca4..e4f2bb6 100644 >> --- a/dlls/ntdll/virtual.c >> +++ b/dlls/ntdll/virtual.c >> @@ -62,6 +62,10 @@ WINE_DECLARE_DEBUG_CHANNEL(module); >> #define MS_SYNC 0 >> #endif >> >> +#ifndef MS_ASYNC >> +#define MS_ASYNC 1 >> +#endif > That's questionable, there's no reason the value would be 1. We defined > MS_SYNC to 0 on the theory that it's better to pass 0 than making up > values, but probably not calling the function at all would be > preferable. > #define MS_ASYNC 1 was taken from my /usr/include/bits/mman.h . The msync manpage does not define the sync behavior of the function when neither MS_SYNC nor MS_ASYNC are set; it probably defaults to safe. In any case, since MS_SYNC is an already-defined constant (four), defining it to 0 is highly misleading. I thought it was bad style to import system headers in Wine code. :) In any case thanks for the response, I'll submit an improved patch.

March 12, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by Frank S Filz

"J. Bruce Fields" <bfields(a)fieldses.org> wrote> > > > Another bit to consider, does lockd provide share reservations for NLM? > > Yes. I don't know if anyone's tested them in recent memory! But it > might be interesting to write a few simple tests for them and hook them > up to this on the server side. (I don't know if they'd be worth > implementing on the client side?) Oh, they would be REALLY NICE to have on the client side... Then we can test NLM_SHARE with an open source client and not only with Windows... Frank

March 11, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by J. Bruce Fields

On Mon, Mar 11, 2013 at 01:25:20PM -0700, Frank S Filz wrote: > > "J. Bruce Fields" <bfields(a)fieldses.org> wrote > > On Mon, Mar 11, 2013 at 04:08:44PM -0400, Jeff Layton wrote: > > > On Mon, 11 Mar 2013 15:36:38 -0400 > > > "J. Bruce Fields" <bfields(a)fieldses.org> wrote: > > > > > It doesn't look like this patch removes any of that old code > though. I > > > > > think it probably should, or there ought to be some consideration > of > > > > > how this new stuff will mesh with it. > > > > > > > > > > I think you have 2 choices here: > > > > > > > > > > 1/ rip out the old share reservation code altogether and require > that > > > > > filesystems mount with -o sharemand or whatever if they want to > allow > > > > > their enforcement > > > > > > > > > > 2/ make knfsd fall back to using the internal share reservation > code > > > > > when the mount option isn't enabled > > > > > > > > > > Personally, I think #1 would be fine, but Bruce may want to weigh > in on > > > > > what he'd prefer. > > > > > > > > #1 sounds good. Clients that use deny bits are few. My preference > > > > would be to return an error to such clients in the case share locks > > > > aren't available. > > > > > > > > (We're a little out of spec there, so I'm not sure which error. I > think > > > > the goal is to notify a human there's a problem with minimal > collateral > > > > damange. > > > > > > > > NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry about that!") would > > > > probably result in an IO error to the application. > > > > > > > > SHARE_DENIED strikes me as unsafe: an application would be in its > rights > > > > not to even check for that e.g. in the case of an exclusive create. > > Hmm, shouldn't the client catch that with a "default" case at least? > > > > > Maybe DELAY? Kind of ridiculous, but blocking the application > > > > indefinitely would probably get someone's attention quickly enough > > > > without doing any damnage.) > > > > > > > > > > I agree that we should return an error, but hadn't considered what > > > error. Given that hardly any NFS clients use them, I'd probably just go > > > with NFS4ERR_SERVERFAULT, and maybe throw a printk or something on the > > > server about enabling share reservations for superblock x:y. > > > > Sounds reasonable. > > If I'm understanding, the suggestion is a mount option to enable share > reservations and if so, they will be mandatory? > > In that case, perhaps we want to keep the existing knfsd code as a > fallback, someone might want to support them, but not have them be > mandatory (if nothing else, you may cause consternation from folks running > pynfs against a default configured knfsd server....). Understood, but the benefit is slight and the cost (in complexity) is rather large. On balance I'd far prefer to get rid of any fallback code entirely. > In the Ganesha project, we provide an internal implementation of share > reservations for when the underlying system can not support them. > > Another bit to consider, does lockd provide share reservations for NLM? Yes. I don't know if anyone's tested them in recent memory! But it might be interesting to write a few simple tests for them and hook them up to this on the server side. (I don't know if they'd be worth implementing on the client side?) --b.

March 11, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by Frank S Filz

"J. Bruce Fields" <bfields(a)fieldses.org> wrote > On Mon, Mar 11, 2013 at 04:08:44PM -0400, Jeff Layton wrote: > > On Mon, 11 Mar 2013 15:36:38 -0400 > > "J. Bruce Fields" <bfields(a)fieldses.org> wrote: > > > > It doesn't look like this patch removes any of that old code though. I > > > > think it probably should, or there ought to be some consideration of > > > > how this new stuff will mesh with it. > > > > > > > > I think you have 2 choices here: > > > > > > > > 1/ rip out the old share reservation code altogether and require that > > > > filesystems mount with -o sharemand or whatever if they want to allow > > > > their enforcement > > > > > > > > 2/ make knfsd fall back to using the internal share reservation code > > > > when the mount option isn't enabled > > > > > > > > Personally, I think #1 would be fine, but Bruce may want to weigh in on > > > > what he'd prefer. > > > > > > #1 sounds good. Clients that use deny bits are few. My preference > > > would be to return an error to such clients in the case share locks > > > aren't available. > > > > > > (We're a little out of spec there, so I'm not sure which error. I think > > > the goal is to notify a human there's a problem with minimal collateral > > > damange. > > > > > > NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry about that!") would > > > probably result in an IO error to the application. > > > > > > SHARE_DENIED strikes me as unsafe: an application would be in its rights > > > not to even check for that e.g. in the case of an exclusive create. Hmm, shouldn't the client catch that with a "default" case at least? > > > Maybe DELAY? Kind of ridiculous, but blocking the application > > > indefinitely would probably get someone's attention quickly enough > > > without doing any damnage.) > > > > > > > I agree that we should return an error, but hadn't considered what > > error. Given that hardly any NFS clients use them, I'd probably just go > > with NFS4ERR_SERVERFAULT, and maybe throw a printk or something on the > > server about enabling share reservations for superblock x:y. > > Sounds reasonable. If I'm understanding, the suggestion is a mount option to enable share reservations and if so, they will be mandatory? In that case, perhaps we want to keep the existing knfsd code as a fallback, someone might want to support them, but not have them be mandatory (if nothing else, you may cause consternation from folks running pynfs against a default configured knfsd server....). In the Ganesha project, we provide an internal implementation of share reservations for when the underlying system can not support them. Another bit to consider, does lockd provide share reservations for NLM? Frank

March 11, 2013

Re: ntdll: Make msync() asynchronous in NtFlushVirtualMemory (resubmit with real name)

by Alexandre Julliard

Mathis Beer <default_357-line(a)yahoo.de> writes: > diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c > index a99bca4..e4f2bb6 100644 > --- a/dlls/ntdll/virtual.c > +++ b/dlls/ntdll/virtual.c > @@ -62,6 +62,10 @@ WINE_DECLARE_DEBUG_CHANNEL(module); > #define MS_SYNC 0 > #endif > > +#ifndef MS_ASYNC > +#define MS_ASYNC 1 > +#endif That's questionable, there's no reason the value would be 1. We defined MS_SYNC to 0 on the theory that it's better to pass 0 than making up values, but probably not calling the function at all would be preferable. -- Alexandre Julliard julliard(a)winehq.org

March 11, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by J. Bruce Fields

On Mon, Mar 11, 2013 at 04:08:44PM -0400, Jeff Layton wrote: > On Mon, 11 Mar 2013 15:36:38 -0400 > "J. Bruce Fields" <bfields(a)fieldses.org> wrote: > > > On Mon, Mar 11, 2013 at 03:05:40PM -0400, Jeff Layton wrote: > > > knfsd has some code already to handle share reservations internally. > > > Nothing outside of knfsd is aware of these reservations, of course so > > > moving to a vfs-level object for it would be a marked improvement. > > > > > > It doesn't look like this patch removes any of that old code though. I > > > think it probably should, or there ought to be some consideration of > > > how this new stuff will mesh with it. > > > > > > I think you have 2 choices here: > > > > > > 1/ rip out the old share reservation code altogether and require that > > > filesystems mount with -o sharemand or whatever if they want to allow > > > their enforcement > > > > > > 2/ make knfsd fall back to using the internal share reservation code > > > when the mount option isn't enabled > > > > > > Personally, I think #1 would be fine, but Bruce may want to weigh in on > > > what he'd prefer. > > > > #1 sounds good. Clients that use deny bits are few. My preference > > would be to return an error to such clients in the case share locks > > aren't available. > > > > (We're a little out of spec there, so I'm not sure which error. I think > > the goal is to notify a human there's a problem with minimal collateral > > damange. > > > > NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry about that!") would > > probably result in an IO error to the application. > > > > SHARE_DENIED strikes me as unsafe: an application would be in its rights > > not to even check for that e.g. in the case of an exclusive create. > > > > Maybe DELAY? Kind of ridiculous, but blocking the application > > indefinitely would probably get someone's attention quickly enough > > without doing any damnage.) > > > > I agree that we should return an error, but hadn't considered what > error. Given that hardly any NFS clients use them, I'd probably just go > with NFS4ERR_SERVERFAULT, and maybe throw a printk or something on the > server about enabling share reservations for superblock x:y. Sounds reasonable. > Pavel, as a side note, you may want to consider adding a patch to hook > this stuff up in the NFS client as well. Definitely. --b.

March 11, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by Jeff Layton

On Mon, 11 Mar 2013 15:36:38 -0400 "J. Bruce Fields" <bfields(a)fieldses.org> wrote: > On Mon, Mar 11, 2013 at 03:05:40PM -0400, Jeff Layton wrote: > > knfsd has some code already to handle share reservations internally. > > Nothing outside of knfsd is aware of these reservations, of course so > > moving to a vfs-level object for it would be a marked improvement. > > > > It doesn't look like this patch removes any of that old code though. I > > think it probably should, or there ought to be some consideration of > > how this new stuff will mesh with it. > > > > I think you have 2 choices here: > > > > 1/ rip out the old share reservation code altogether and require that > > filesystems mount with -o sharemand or whatever if they want to allow > > their enforcement > > > > 2/ make knfsd fall back to using the internal share reservation code > > when the mount option isn't enabled > > > > Personally, I think #1 would be fine, but Bruce may want to weigh in on > > what he'd prefer. > > #1 sounds good. Clients that use deny bits are few. My preference > would be to return an error to such clients in the case share locks > aren't available. > > (We're a little out of spec there, so I'm not sure which error. I think > the goal is to notify a human there's a problem with minimal collateral > damange. > > NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry about that!") would > probably result in an IO error to the application. > > SHARE_DENIED strikes me as unsafe: an application would be in its rights > not to even check for that e.g. in the case of an exclusive create. > > Maybe DELAY? Kind of ridiculous, but blocking the application > indefinitely would probably get someone's attention quickly enough > without doing any damnage.) > I agree that we should return an error, but hadn't considered what error. Given that hardly any NFS clients use them, I'd probably just go with NFS4ERR_SERVERFAULT, and maybe throw a printk or something on the server about enabling share reservations for superblock x:y. Pavel, as a side note, you may want to consider adding a patch to hook this stuff up in the NFS client as well. -- Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: msvcp60: Avoid signed-unsigned integer comparisons

by Andrew Talbot

larmbr zhan wrote: > On Sun, Mar 10, 2013 at 5:48 AM, Andrew Talbot > <andrew.talbot(a)talbotville.com> wrote: >> msvcp60: Avoid signed-unsigned integer comparisons. > > > Hi, Andrew Talbot. > > I find that you are working on these "Avoid signed-unsigned integer > comparisons" things recently. > > I DO agree on that using _ unsigned int _ instead of _ int _(which > implies _ signed int _). > But I think using size_t maybe more standard-compliant, more > efficient, and less bug-prone, . > > [...] Hi, Zhan JIanyu, I would say that size_t (and ptrdiff_t) are most suitable for "memsizes" that are open-ended and could potentially have huge values. However, where it is clear that gigabytes will never be involved nor billions of items, there is nothing more efficient than an int (signed or unsigned), since it has the same width as the processor. For real-world quantities, such as "width" and "height", I would prefer to use signed intS where possible, even for non-negative commodities. I would prefer unsigned intS for bit arrays. Regard, -- Andy.

March 11, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by J. Bruce Fields

On Mon, Mar 11, 2013 at 03:05:40PM -0400, Jeff Layton wrote: > knfsd has some code already to handle share reservations internally. > Nothing outside of knfsd is aware of these reservations, of course so > moving to a vfs-level object for it would be a marked improvement. > > It doesn't look like this patch removes any of that old code though. I > think it probably should, or there ought to be some consideration of > how this new stuff will mesh with it. > > I think you have 2 choices here: > > 1/ rip out the old share reservation code altogether and require that > filesystems mount with -o sharemand or whatever if they want to allow > their enforcement > > 2/ make knfsd fall back to using the internal share reservation code > when the mount option isn't enabled > > Personally, I think #1 would be fine, but Bruce may want to weigh in on > what he'd prefer. #1 sounds good. Clients that use deny bits are few. My preference would be to return an error to such clients in the case share locks aren't available. (We're a little out of spec there, so I'm not sure which error. I think the goal is to notify a human there's a problem with minimal collateral damange. NFS4ERR_SERVERFAULT ("I'm a buggy server, sorry about that!") would probably result in an IO error to the application. SHARE_DENIED strikes me as unsafe: an application would be in its rights not to even check for that e.g. in the case of an exclusive create. Maybe DELAY? Kind of ridiculous, but blocking the application indefinitely would probably get someone's attention quickly enough without doing any damnage.) --b.

March 11, 2013

Re: [PATCH v3 2/7] vfs: Add O_DENYREAD/WRITE flags support for open syscall

by Jeff Layton

On Mon, 11 Mar 2013 22:57:27 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > 2013/3/11 Jeff Layton <jlayton(a)redhat.com>: > > On Thu, 28 Feb 2013 19:25:28 +0400 > > Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > > > >> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are > >> translated to flock's flags: > >> > >> !O_DENYREAD -> LOCK_READ > >> !O_DENYWRITE -> LOCK_WRITE > >> O_DENYMAND -> LOCK_MAND > >> > >> and set through flock_lock_file on a file. > >> > >> This change affects opens that use O_DENYMAND flag - all other > >> native Linux opens don't care about these flags. It allow us to > >> enable this feature for applications that need it (e.g. NFS and > >> Samba servers that export the same directory for Windows clients, > >> or Wine applications that access the same files simultaneously). > >> > >> Create codepath is slightly changed to prevent data races on > >> newely created files: when open with O_CREAT can return with -ETXTBSY > >> error for successfully created files due to a deny lock set by > >> another task. > >> > >> Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > >> --- > >> fs/locks.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++------ > >> fs/namei.c | 44 ++++++++++++++++++-- > >> include/linux/fs.h | 6 +++ > >> 3 files changed, 151 insertions(+), 15 deletions(-) > >> > >> diff --git a/fs/locks.c b/fs/locks.c > >> index a94e331..0cc7d1b 100644 > >> --- a/fs/locks.c > >> +++ b/fs/locks.c > >> @@ -605,20 +605,81 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s > >> return (locks_conflict(caller_fl, sys_fl)); > >> } > >> > >> -/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific > >> - * checking before calling the locks_conflict(). > >> +static unsigned int > >> +deny_flags_to_cmd(unsigned int flags) > >> +{ > >> + unsigned int cmd = LOCK_MAND; > >> + > >> + if (!(flags & O_DENYREAD)) > >> + cmd |= LOCK_READ; > >> + if (!(flags & O_DENYWRITE)) > >> + cmd |= LOCK_WRITE; > >> + > >> + return cmd; > >> +} > >> + > >> +/* > >> + * locks_mand_conflict - Determine if there's a share reservation conflict > >> + * @caller_fl: lock we're attempting to acquire > >> + * @sys_fl: lock already present on system that we're checking against > >> + * > >> + * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE > >> + * tell us whether the reservation allows other readers and writers. > >> + * > >> + * We only check against other LOCK_MAND locks, so applications that want to > >> + * use share mode locking will only conflict against one another. "normal" > >> + * applications that open files won't be affected by and won't themselves > >> + * affect the share reservations. > >> */ > >> -static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) > >> +static int > >> +locks_mand_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) > >> { > >> - /* FLOCK locks referring to the same filp do not conflict with > >> + unsigned char caller_type = caller_fl->fl_type; > >> + unsigned char sys_type = sys_fl->fl_type; > >> + fmode_t caller_fmode = caller_fl->fl_file->f_mode; > >> + fmode_t sys_fmode = sys_fl->fl_file->f_mode; > >> + > >> + /* they can only conflict if they're both LOCK_MAND */ > >> + if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND)) > >> + return 0; > >> + > >> + if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ)) > >> + return 1; > >> + if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE)) > >> + return 1; > >> + if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ)) > >> + return 1; > >> + if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE)) > >> + return 1; > >> + > >> + return 0; > >> +} > >> + > >> +/* > >> + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific checking > >> + * before calling the locks_conflict(). Boolean is_mand indicates whether > >> + * we should use a share reservation scheme or not. > >> + */ > >> +static int > >> +flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl, > >> + bool is_mand) > > > > I'm not sure you really need to add this new is_mand bool. Won't that > > be equivalent to (caller_fl->fl_type & LOCK_MAND)? > > This function is already used by flock system call that can pass > LOCK_MAND flag to caller_fl->fl_type. I don't want to affect existing > flock behavior by introduing new denylocking strategy - so, we need to > let flock_locks_conflict function know if we are in flock or open > codepath - in open codepath it will call locks_mand_conflict to check > if there is any other open that prevents us. > Right, but if you move to a mount option for this, then enforcing these locks in the flock() codepath should be ok. It seems reasonable that anyone who wants enforcement of O_DENY* would want to enforce LOCK_MAND locks as well. -- Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: [PATCH v3 7/7] NFSD: Pass share reservations flags to VFS

by Jeff Layton

On Thu, 28 Feb 2013 19:25:33 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > that maps them into O_DENY flags and make them visible for > applications that use O_DENYMAND opens. > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > --- > fs/locks.c | 1 + > fs/nfsd/nfs4state.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/fs/locks.c b/fs/locks.c > index 0cc7d1b..593d464 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -874,6 +874,7 @@ deny_lock_file(struct file *filp) > locks_free_lock(lock); > return error; > } > +EXPORT_SYMBOL(deny_lock_file); > > static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock) > { > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index ac8ed96c..766256a 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -476,6 +476,19 @@ test_deny(u32 access, struct nfs4_ol_stateid *stp) > return test_bit(access, &stp->st_deny_bmap); > } > > +static int nfs4_deny_to_odeny(u32 access) > +{ > + switch (access & NFS4_SHARE_DENY_BOTH) { > + case NFS4_SHARE_DENY_READ: > + return O_DENYMAND | O_DENYREAD; > + case NFS4_SHARE_DENY_WRITE: > + return O_DENYWRITE | O_DENYMAND; > + case NFS4_SHARE_DENY_BOTH: > + return O_DENYREAD | O_DENYWRITE | O_DENYMAND; > + } > + return O_DENYMAND; > +} > + > static int nfs4_access_to_omode(u32 access) > { > switch (access & NFS4_SHARE_ACCESS_BOTH) { > @@ -2793,6 +2806,21 @@ nfsd4_truncate(struct svc_rqst *rqstp, struct svc_fh *fh, > } > > static __be32 > +nfs4_vfs_set_deny(struct nfs4_file *fp, unsigned long share_access, > + unsigned long deny_access) > +{ > + int oflag, rc; > + __be32 status = nfs_ok; > + > + oflag = nfs4_access_to_omode(share_access); > + fp->fi_fds[oflag]->f_flags |= nfs4_deny_to_odeny(deny_access); > + rc = deny_lock_file(fp->fi_fds[oflag]); > + if (rc) > + status = nfserrno(rc); > + return status; > +} > + > +static __be32 > nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *cur_fh, struct nfs4_ol_stateid *stp, struct nfsd4_open *open) > { > u32 op_share_access = open->op_share_access; > @@ -2813,6 +2841,14 @@ nfs4_upgrade_open(struct svc_rqst *rqstp, struct nfs4_file *fp, struct svc_fh *c > } > return status; > } > + status = nfs4_vfs_set_deny(fp, op_share_access, open->op_share_deny); > + if (status) { > + if (new_access) { > + int oflag = nfs4_access_to_omode(op_share_access); > + nfs4_file_put_access(fp, oflag); > + } > + return status; > + } > /* remember the open */ > set_access(op_share_access, stp); > set_deny(open->op_share_deny, stp); > @@ -3046,7 +3082,7 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf > > /* > * OPEN the file, or upgrade an existing OPEN. > - * If truncate fails, the OPEN fails. > + * If truncate or setting deny fails, the OPEN fails. > */ > if (stp) { > /* Stateid was found, this is an OPEN upgrade */ > @@ -3060,6 +3096,10 @@ nfsd4_process_open2(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nf > status = nfsd4_truncate(rqstp, current_fh, open); > if (status) > goto out; > + status = nfs4_vfs_set_deny(fp, open->op_share_access, > + open->op_share_deny); > + if (status) > + goto out; > stp = open->op_stp; > open->op_stp = NULL; > init_open_stateid(stp, fp, open); > @@ -3758,6 +3798,10 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp, > } > nfs4_stateid_downgrade(stp, od->od_share_access); > > + status = nfs4_vfs_set_deny(stp->st_file, od->od_share_access, > + od->od_share_deny); > + if (status) > + goto out; > reset_union_bmap_deny(od->od_share_deny, stp); > > update_stateid(&stp->st_stid.sc_stateid); knfsd has some code already to handle share reservations internally. Nothing outside of knfsd is aware of these reservations, of course so moving to a vfs-level object for it would be a marked improvement. It doesn't look like this patch removes any of that old code though. I think it probably should, or there ought to be some consideration of how this new stuff will mesh with it. I think you have 2 choices here: 1/ rip out the old share reservation code altogether and require that filesystems mount with -o sharemand or whatever if they want to allow their enforcement 2/ make knfsd fall back to using the internal share reservation code when the mount option isn't enabled Personally, I think #1 would be fine, but Bruce may want to weigh in on what he'd prefer. -- Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: [PATCH v3 5/7] CIFS: Translate SHARING_VIOLATION to -ETXTBSY error code for SMB2

by Pavel Shilovsky

2013/3/11 Jeff Layton <jlayton(a)redhat.com>: > On Thu, 28 Feb 2013 19:25:31 +0400 > Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > >> to make it match CIFS and VFS variants. >> >> Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> >> --- >> fs/cifs/smb2maperror.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/cifs/smb2maperror.c b/fs/cifs/smb2maperror.c >> index 494c912..11e589e 100644 >> --- a/fs/cifs/smb2maperror.c >> +++ b/fs/cifs/smb2maperror.c >> @@ -356,7 +356,7 @@ static const struct status_to_posix_error smb2_error_map_table[] = { >> {STATUS_PORT_CONNECTION_REFUSED, -ECONNREFUSED, >> "STATUS_PORT_CONNECTION_REFUSED"}, >> {STATUS_INVALID_PORT_HANDLE, -EIO, "STATUS_INVALID_PORT_HANDLE"}, >> - {STATUS_SHARING_VIOLATION, -EBUSY, "STATUS_SHARING_VIOLATION"}, >> + {STATUS_SHARING_VIOLATION, -ETXTBSY, "STATUS_SHARING_VIOLATION"}, >> {STATUS_QUOTA_EXCEEDED, -EDQUOT, "STATUS_QUOTA_EXCEEDED"}, >> {STATUS_INVALID_PAGE_PROTECTION, -EIO, >> "STATUS_INVALID_PAGE_PROTECTION"}, > > Actually, I think Sachin is converting the CIFS > STATUS_SHARING_VIOLATION to translate to EBUSY, since that seems to > better reflect the situation. I'd suggest dropping this patch, unless > you have a specific need for this error return here. Yes, I am ok to drop this patch - accoring to the previous discussion in linux-cifs@ it is no suitable. -- Best regards, Pavel Shilovsky.

March 11, 2013

Re: [PATCH v3 2/7] vfs: Add O_DENYREAD/WRITE flags support for open syscall

by Pavel Shilovsky

2013/3/11 Jeff Layton <jlayton(a)redhat.com>: > On Thu, 28 Feb 2013 19:25:28 +0400 > Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > >> If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are >> translated to flock's flags: >> >> !O_DENYREAD -> LOCK_READ >> !O_DENYWRITE -> LOCK_WRITE >> O_DENYMAND -> LOCK_MAND >> >> and set through flock_lock_file on a file. >> >> This change affects opens that use O_DENYMAND flag - all other >> native Linux opens don't care about these flags. It allow us to >> enable this feature for applications that need it (e.g. NFS and >> Samba servers that export the same directory for Windows clients, >> or Wine applications that access the same files simultaneously). >> >> Create codepath is slightly changed to prevent data races on >> newely created files: when open with O_CREAT can return with -ETXTBSY >> error for successfully created files due to a deny lock set by >> another task. >> >> Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> >> --- >> fs/locks.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++------ >> fs/namei.c | 44 ++++++++++++++++++-- >> include/linux/fs.h | 6 +++ >> 3 files changed, 151 insertions(+), 15 deletions(-) >> >> diff --git a/fs/locks.c b/fs/locks.c >> index a94e331..0cc7d1b 100644 >> --- a/fs/locks.c >> +++ b/fs/locks.c >> @@ -605,20 +605,81 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s >> return (locks_conflict(caller_fl, sys_fl)); >> } >> >> -/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific >> - * checking before calling the locks_conflict(). >> +static unsigned int >> +deny_flags_to_cmd(unsigned int flags) >> +{ >> + unsigned int cmd = LOCK_MAND; >> + >> + if (!(flags & O_DENYREAD)) >> + cmd |= LOCK_READ; >> + if (!(flags & O_DENYWRITE)) >> + cmd |= LOCK_WRITE; >> + >> + return cmd; >> +} >> + >> +/* >> + * locks_mand_conflict - Determine if there's a share reservation conflict >> + * @caller_fl: lock we're attempting to acquire >> + * @sys_fl: lock already present on system that we're checking against >> + * >> + * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE >> + * tell us whether the reservation allows other readers and writers. >> + * >> + * We only check against other LOCK_MAND locks, so applications that want to >> + * use share mode locking will only conflict against one another. "normal" >> + * applications that open files won't be affected by and won't themselves >> + * affect the share reservations. >> */ >> -static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) >> +static int >> +locks_mand_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) >> { >> - /* FLOCK locks referring to the same filp do not conflict with >> + unsigned char caller_type = caller_fl->fl_type; >> + unsigned char sys_type = sys_fl->fl_type; >> + fmode_t caller_fmode = caller_fl->fl_file->f_mode; >> + fmode_t sys_fmode = sys_fl->fl_file->f_mode; >> + >> + /* they can only conflict if they're both LOCK_MAND */ >> + if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND)) >> + return 0; >> + >> + if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ)) >> + return 1; >> + if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE)) >> + return 1; >> + if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ)) >> + return 1; >> + if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE)) >> + return 1; >> + >> + return 0; >> +} >> + >> +/* >> + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific checking >> + * before calling the locks_conflict(). Boolean is_mand indicates whether >> + * we should use a share reservation scheme or not. >> + */ >> +static int >> +flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl, >> + bool is_mand) > > I'm not sure you really need to add this new is_mand bool. Won't that > be equivalent to (caller_fl->fl_type & LOCK_MAND)? This function is already used by flock system call that can pass LOCK_MAND flag to caller_fl->fl_type. I don't want to affect existing flock behavior by introduing new denylocking strategy - so, we need to let flock_locks_conflict function know if we are in flock or open codepath - in open codepath it will call locks_mand_conflict to check if there is any other open that prevents us. > >> +{ >> + /* >> + * FLOCK locks referring to the same filp do not conflict with >> * each other. >> */ >> - if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file)) >> - return (0); >> - if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND)) >> + if (!IS_FLOCK(sys_fl)) >> + return 0; >> + if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND)) { >> + if (is_mand) >> + return locks_mand_conflict(caller_fl, sys_fl); >> + else >> + return 0; >> + } >> + if (caller_fl->fl_file == sys_fl->fl_file) >> return 0; >> >> - return (locks_conflict(caller_fl, sys_fl)); >> + return locks_conflict(caller_fl, sys_fl); >> } >> >> void >> @@ -697,14 +758,19 @@ static int posix_locks_deadlock(struct file_lock *caller_fl, >> return 0; >> } >> >> -/* Try to create a FLOCK lock on filp. We always insert new FLOCK locks >> +/* >> + * Try to create a FLOCK lock on filp. We always insert new FLOCK locks >> * after any leases, but before any posix locks. >> * >> * Note that if called with an FL_EXISTS argument, the caller may determine >> * whether or not a lock was successfully freed by testing the return >> * value for -ENOENT. >> + * >> + * Take @is_conflict callback that determines how to check if locks have >> + * conflicts or not. >> */ >> -static int flock_lock_file(struct file *filp, struct file_lock *request) >> +static int >> +flock_lock_file(struct file *filp, struct file_lock *request, bool is_mand) > > Ditto here on the is_mand bool. I think you can determine that from > request->fl_type. Right? The same suggestions are applied to this place too. > >> { >> struct file_lock *new_fl = NULL; >> struct file_lock **before; >> @@ -760,7 +826,7 @@ find_conflict: >> break; >> if (IS_LEASE(fl)) >> continue; >> - if (!flock_locks_conflict(request, fl)) >> + if (!flock_locks_conflict(request, fl, is_mand)) >> continue; >> error = -EAGAIN; >> if (!(request->fl_flags & FL_SLEEP)) >> @@ -783,6 +849,32 @@ out: >> return error; >> } >> >> +/* >> + * Determine if a file is allowed to be opened with specified access and deny >> + * modes. Lock the file and return 0 if checks passed, otherwise return a error >> + * code. >> + */ >> +int >> +deny_lock_file(struct file *filp) >> +{ >> + struct file_lock *lock; >> + int error = 0; >> + >> + if (!(filp->f_flags & O_DENYMAND)) >> + return error; >> + >> + error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags)); >> + if (error) >> + return error; >> + >> + error = flock_lock_file(filp, lock, true); >> + if (error == -EAGAIN) >> + error = -ETXTBSY; >> + > > I think EBUSY would be a better return code here. ETXTBSY is returned > in more specific circumstances -- mostly when you're opening a file for > write that is being executed. Yes, I agree. This work was done before the discussion in linux-cifs@ about a error code for STATUS_SHARING_VIOLATION. > >> + locks_free_lock(lock); >> + return error; >> +} >> + >> static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock) >> { >> struct file_lock *fl; >> @@ -1589,7 +1681,7 @@ int flock_lock_file_wait(struct file *filp, struct file_lock *fl) >> int error; >> might_sleep(); >> for (;;) { >> - error = flock_lock_file(filp, fl); >> + error = flock_lock_file(filp, fl, false); >> if (error != FILE_LOCK_DEFERRED) >> break; >> error = wait_event_interruptible(fl->fl_wait, !fl->fl_next); >> diff --git a/fs/namei.c b/fs/namei.c >> index 43a97ee..c1f7e08 100644 >> --- a/fs/namei.c >> +++ b/fs/namei.c >> @@ -2559,9 +2559,14 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry, >> * here. >> */ >> error = may_open(&file->f_path, acc_mode, open_flag); >> - if (error) >> + if (error) { >> fput(file); >> + goto out; >> + } >> >> + error = deny_lock_file(file); >> + if (error) >> + fput(file); >> out: >> dput(dentry); >> return error; >> @@ -2771,9 +2776,9 @@ retry_lookup: >> } >> mutex_lock(&dir->d_inode->i_mutex); >> error = lookup_open(nd, path, file, op, got_write, opened); >> - mutex_unlock(&dir->d_inode->i_mutex); >> >> if (error <= 0) { >> + mutex_unlock(&dir->d_inode->i_mutex); >> if (error) >> goto out; >> >> @@ -2791,8 +2796,32 @@ retry_lookup: >> will_truncate = false; >> acc_mode = MAY_OPEN; >> path_to_nameidata(path, nd); >> - goto finish_open_created; >> + >> + /* >> + * Unlock parent i_mutex later when the open finishes - prevent >> + * races when a file can be locked with a deny lock by another >> + * task that opens the file. >> + */ >> + error = may_open(&nd->path, acc_mode, open_flag); >> + if (error) { >> + mutex_unlock(&dir->d_inode->i_mutex); >> + goto out; >> + } >> + file->f_path.mnt = nd->path.mnt; >> + error = finish_open(file, nd->path.dentry, NULL, opened); >> + if (error) { >> + mutex_unlock(&dir->d_inode->i_mutex); >> + if (error == -EOPENSTALE) >> + goto stale_open; >> + goto out; >> + } >> + error = deny_lock_file(file); >> + mutex_unlock(&dir->d_inode->i_mutex); >> + if (error) >> + goto exit_fput; >> + goto opened; >> } >> + mutex_unlock(&dir->d_inode->i_mutex); >> >> /* >> * create/update audit record if it already exists. >> @@ -2885,6 +2914,15 @@ finish_open_created: >> goto stale_open; >> goto out; >> } >> + /* >> + * Lock parent i_mutex to prevent races with deny locks on newely >> + * created files. >> + */ >> + mutex_lock(&dir->d_inode->i_mutex); >> + error = deny_lock_file(file); >> + mutex_unlock(&dir->d_inode->i_mutex); >> + if (error) >> + goto exit_fput; >> opened: >> error = open_check_o_direct(file); >> if (error) >> diff --git a/include/linux/fs.h b/include/linux/fs.h >> index 7617ee0..347e1de 100644 >> --- a/include/linux/fs.h >> +++ b/include/linux/fs.h >> @@ -1005,6 +1005,7 @@ extern int lease_modify(struct file_lock **, int); >> extern int lock_may_read(struct inode *, loff_t start, unsigned long count); >> extern int lock_may_write(struct inode *, loff_t start, unsigned long count); >> extern void locks_delete_block(struct file_lock *waiter); >> +extern int deny_lock_file(struct file *); >> extern void lock_flocks(void); >> extern void unlock_flocks(void); >> #else /* !CONFIG_FILE_LOCKING */ >> @@ -1153,6 +1154,11 @@ static inline void locks_delete_block(struct file_lock *waiter) >> { >> } >> >> +static inline int deny_lock_file(struct file *filp) >> +{ >> + return 0; >> +} >> + >> static inline void lock_flocks(void) >> { >> } > > > -- > Jeff Layton <jlayton(a)redhat.com> > -- > To unsubscribe from this list: send the line "unsubscribe linux-cifs" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best regards, Pavel Shilovsky.

March 11, 2013

Re: [PATCH v3 6/7] NFSv4: Add O_DENY* open flags support

by Jeff Layton

On Thu, 28 Feb 2013 19:25:32 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > by passing these flags to NFSv4 open request. > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > --- > fs/nfs/nfs4xdr.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c > index 26b1439..58ddc74 100644 > --- a/fs/nfs/nfs4xdr.c > +++ b/fs/nfs/nfs4xdr.c > @@ -1325,7 +1325,8 @@ static void encode_lookup(struct xdr_stream *xdr, const struct qstr *name, struc > encode_string(xdr, name->len, name->name); > } > > -static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode) > +static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode, > + int open_flags) > { > __be32 *p; > > @@ -1343,7 +1344,22 @@ static void encode_share_access(struct xdr_stream *xdr, fmode_t fmode) > default: > *p++ = cpu_to_be32(0); > } > - *p = cpu_to_be32(0); /* for linux, share_deny = 0 always */ > + if (open_flags & O_DENYMAND) { As Bruce mentioned, I think a mount option to enable this on a per-fs basis would be a better approach than this new O_DENYMAND flag. > + switch (open_flags & (O_DENYREAD|O_DENYWRITE)) { > + case O_DENYREAD: > + *p = cpu_to_be32(NFS4_SHARE_DENY_READ); > + break; > + case O_DENYWRITE: > + *p = cpu_to_be32(NFS4_SHARE_DENY_WRITE); > + break; > + case O_DENYREAD|O_DENYWRITE: > + *p = cpu_to_be32(NFS4_SHARE_DENY_BOTH); > + break; > + default: > + *p = cpu_to_be32(0); > + } > + } else > + *p = cpu_to_be32(0); > } > > static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_openargs *arg) > @@ -1354,7 +1370,7 @@ static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_opena > * owner 4 = 32 > */ > encode_nfs4_seqid(xdr, arg->seqid); > - encode_share_access(xdr, arg->fmode); > + encode_share_access(xdr, arg->fmode, arg->open_flags); > p = reserve_space(xdr, 36); > p = xdr_encode_hyper(p, arg->clientid); > *p++ = cpu_to_be32(24); > @@ -1491,7 +1507,7 @@ static void encode_open_downgrade(struct xdr_stream *xdr, const struct nfs_close > encode_op_hdr(xdr, OP_OPEN_DOWNGRADE, decode_open_downgrade_maxsz, hdr); > encode_nfs4_stateid(xdr, arg->stateid); > encode_nfs4_seqid(xdr, arg->seqid); > - encode_share_access(xdr, arg->fmode); > + encode_share_access(xdr, arg->fmode, 0); > } > > static void Other than that, this seems reasonable. Acked-by: Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: [PATCH v3 4/7] CIFS: Use NT_CREATE_ANDX command for forcemand mounts

by Jeff Layton

On Thu, 28 Feb 2013 19:25:30 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > forcemand mount option now lets us use Windows mandatory style of > byte-range locks even if server supports posix ones - switches on > Windows locking mechanism. Share flags is another locking mehanism > provided by Windows semantic that can be used by NT_CREATE_ANDX > command. This patch combines all Windows locking mechanism in one > mount option by using NT_CREATE_ANDX to open files if forcemand is on. > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > --- > fs/cifs/dir.c | 1 + > fs/cifs/file.c | 6 ++++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 6975072..139c8bc 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -217,6 +217,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid, > } > > if (tcon->unix_ext && cap_unix(tcon->ses) && !tcon->broken_posix_open && > + ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) && > (CIFS_UNIX_POSIX_PATH_OPS_CAP & > le64_to_cpu(tcon->fsUnixInfo.Capability))) { > rc = cifs_posix_open(full_path, &newinode, inode->i_sb, mode, > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index 3ad484c..05191da 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -454,8 +454,9 @@ int cifs_open(struct inode *inode, struct file *file) > else > oplock = 0; > > - if (!tcon->broken_posix_open && tcon->unix_ext && > - cap_unix(tcon->ses) && (CIFS_UNIX_POSIX_PATH_OPS_CAP & > + if (!tcon->broken_posix_open && tcon->unix_ext && cap_unix(tcon->ses) > + && ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) && > + (CIFS_UNIX_POSIX_PATH_OPS_CAP & > le64_to_cpu(tcon->fsUnixInfo.Capability))) { > /* can not refresh inode info since size could be stale */ > rc = cifs_posix_open(full_path, &inode, inode->i_sb, > @@ -623,6 +624,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush) > oplock = 0; > > if (tcon->unix_ext && cap_unix(tcon->ses) && > + ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NOPOSIXBRL) == 0) && > (CIFS_UNIX_POSIX_PATH_OPS_CAP & > le64_to_cpu(tcon->fsUnixInfo.Capability))) { > /* Sounds reasonable... Acked-by: Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: [PATCH v3 3/7] CIFS: Add O_DENY* open flags support

by Jeff Layton

On Thu, 28 Feb 2013 19:25:29 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > Make CIFSSMBOpen take share_flags as a parm that allows us > to pass new O_DENY* flags to the server. > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > --- > fs/cifs/cifsacl.c | 6 ++++-- > fs/cifs/cifsglob.h | 12 +++++++++++- > fs/cifs/cifsproto.h | 9 +++++---- > fs/cifs/cifssmb.c | 47 +++++++++++++++++++++++++---------------------- > fs/cifs/dir.c | 13 ++++++++----- > fs/cifs/file.c | 12 ++++++++---- > fs/cifs/inode.c | 11 ++++++----- > fs/cifs/link.c | 10 +++++----- > fs/cifs/readdir.c | 2 +- > fs/cifs/smb1ops.c | 15 ++++++++------- > fs/cifs/smb2file.c | 10 +++++----- > fs/cifs/smb2inode.c | 4 ++-- > fs/cifs/smb2ops.c | 10 ++++++---- > fs/cifs/smb2pdu.c | 6 +++--- > fs/cifs/smb2proto.h | 14 ++++++++------ > 15 files changed, 105 insertions(+), 76 deletions(-) > > diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c > index 5cbd00e..222b989 100644 > --- a/fs/cifs/cifsacl.c > +++ b/fs/cifs/cifsacl.c > @@ -891,7 +891,8 @@ static struct cifs_ntsd *get_cifs_acl_by_path(struct cifs_sb_info *cifs_sb, > create_options |= CREATE_OPEN_BACKUP_INTENT; > > rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, READ_CONTROL, > - create_options, &fid, &oplock, NULL, cifs_sb->local_nls, > + FILE_SHARE_ALL, create_options, &fid, &oplock, > + NULL, cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > if (!rc) { > rc = CIFSSMBGetCIFSACL(xid, tcon, fid, &pntsd, pacllen); > @@ -952,7 +953,8 @@ int set_cifs_acl(struct cifs_ntsd *pnntsd, __u32 acllen, > access_flags = WRITE_DAC; > > rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, access_flags, > - create_options, &fid, &oplock, NULL, cifs_sb->local_nls, > + FILE_SHARE_ALL, create_options, &fid, &oplock, > + NULL, cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > if (rc) { > cERROR(1, "Unable to open file to set ACL"); > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index e6899ce..f1d62ed 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -310,7 +310,7 @@ struct smb_version_operations { > struct cifs_sb_info *); > /* open a file for non-posix mounts */ > int (*open)(const unsigned int, struct cifs_tcon *, const char *, int, > - int, int, struct cifs_fid *, __u32 *, FILE_ALL_INFO *, > + int, int, int, struct cifs_fid *, __u32 *, FILE_ALL_INFO *, > struct cifs_sb_info *); > /* set fid protocol-specific info */ > void (*set_fid)(struct cifsFileInfo *, struct cifs_fid *, __u32); > @@ -947,6 +947,16 @@ struct cifsFileInfo { > struct work_struct oplock_break; /* work for oplock breaks */ > }; > > +#define CIFS_DENY_RW_FLAGS_SHIFT 22 > +#define CIFS_DENY_DEL_FLAG_SHIFT 23 > + > +static inline int cifs_get_share_flags(unsigned int flags) > +{ > + return (flags & O_DENYMAND) ? > + (((~(flags >> CIFS_DENY_RW_FLAGS_SHIFT)) & 3) | > + ((~(flags >> CIFS_DENY_DEL_FLAG_SHIFT)) & 4)) : FILE_SHARE_ALL; > +} > + > struct cifs_io_parms { > __u16 netfid; > #ifdef CONFIG_CIFS_SMB2 > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h > index 1988c1b..9661607 100644 > --- a/fs/cifs/cifsproto.h > +++ b/fs/cifs/cifsproto.h > @@ -361,10 +361,11 @@ extern int CIFSSMBQueryReparseLinkInfo(const unsigned int xid, > const struct nls_table *nls_codepage); > #endif /* temporarily unused until cifs_symlink fixed */ > extern int CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon, > - const char *fileName, const int disposition, > - const int access_flags, const int omode, > - __u16 *netfid, int *pOplock, FILE_ALL_INFO *, > - const struct nls_table *nls_codepage, int remap); > + const char *file_name, const int disposition, > + const int access_flags, const int share_flags, > + const int omode, __u16 *netfid, int *oplock, > + FILE_ALL_INFO *, const struct nls_table *nls_codepage, > + int remap); Yuck...maybe it's time for a struct cifs_openargs? At the very least, while you're in here, it would be good to just pass in a cifs_sb instead of a nls_table and a remap flag. > extern int SMBLegacyOpen(const unsigned int xid, struct cifs_tcon *tcon, > const char *fileName, const int disposition, > const int access_flags, const int omode, > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c > index 76d0d29..9c4632f 100644 > --- a/fs/cifs/cifssmb.c > +++ b/fs/cifs/cifssmb.c > @@ -1289,10 +1289,11 @@ OldOpenRetry: > > int > CIFSSMBOpen(const unsigned int xid, struct cifs_tcon *tcon, > - const char *fileName, const int openDisposition, > - const int access_flags, const int create_options, __u16 *netfid, > - int *pOplock, FILE_ALL_INFO *pfile_info, > - const struct nls_table *nls_codepage, int remap) > + const char *file_name, const int disposition, > + const int access_flags, const int share_flags, > + const int create_options, __u16 *netfid, int *oplock, > + FILE_ALL_INFO *file_info, const struct nls_table *nls_codepage, > + int remap) > { > int rc = -EACCES; > OPEN_REQ *pSMB = NULL; > @@ -1313,26 +1314,28 @@ openRetry: > count = 1; /* account for one byte pad to word boundary */ > name_len = > cifsConvertToUTF16((__le16 *) (pSMB->fileName + 1), > - fileName, PATH_MAX, nls_codepage, remap); > + file_name, PATH_MAX, nls_codepage, > + remap); > name_len++; /* trailing null */ > name_len *= 2; > pSMB->NameLength = cpu_to_le16(name_len); > } else { /* BB improve check for buffer overruns BB */ > count = 0; /* no pad */ > - name_len = strnlen(fileName, PATH_MAX); > + name_len = strnlen(file_name, PATH_MAX); > name_len++; /* trailing null */ > pSMB->NameLength = cpu_to_le16(name_len); > - strncpy(pSMB->fileName, fileName, name_len); > + strncpy(pSMB->fileName, file_name, name_len); > } > - if (*pOplock & REQ_OPLOCK) > + if (*oplock & REQ_OPLOCK) > pSMB->OpenFlags = cpu_to_le32(REQ_OPLOCK); > - else if (*pOplock & REQ_BATCHOPLOCK) > + else if (*oplock & REQ_BATCHOPLOCK) > pSMB->OpenFlags = cpu_to_le32(REQ_BATCHOPLOCK); > pSMB->DesiredAccess = cpu_to_le32(access_flags); > pSMB->AllocationSize = 0; > - /* set file as system file if special file such > - as fifo and server expecting SFU style and > - no Unix extensions */ > + /* > + * set file as system file if special file such as fifo and server > + * expecting SFU style and no Unix extensions > + */ > if (create_options & CREATE_OPTION_SPECIAL) > pSMB->FileAttributes = cpu_to_le32(ATTR_SYSTEM); > else > @@ -1347,8 +1350,8 @@ openRetry: > if (create_options & CREATE_OPTION_READONLY) > pSMB->FileAttributes |= cpu_to_le32(ATTR_READONLY); > > - pSMB->ShareAccess = cpu_to_le32(FILE_SHARE_ALL); > - pSMB->CreateDisposition = cpu_to_le32(openDisposition); > + pSMB->ShareAccess = cpu_to_le32(share_flags); > + pSMB->CreateDisposition = cpu_to_le32(disposition); > pSMB->CreateOptions = cpu_to_le32(create_options & CREATE_OPTIONS_MASK); > /* BB Expirement with various impersonation levels and verify */ > pSMB->ImpersonationLevel = cpu_to_le32(SECURITY_IMPERSONATION); > @@ -1366,20 +1369,20 @@ openRetry: > if (rc) { > cFYI(1, "Error in Open = %d", rc); > } else { > - *pOplock = pSMBr->OplockLevel; /* 1 byte no need to le_to_cpu */ > + *oplock = pSMBr->OplockLevel; /* 1 byte no need to le_to_cpu */ > *netfid = pSMBr->Fid; /* cifs fid stays in le */ > /* Let caller know file was created so we can set the mode. */ > /* Do we care about the CreateAction in any other cases? */ > if (cpu_to_le32(FILE_CREATE) == pSMBr->CreateAction) > - *pOplock |= CIFS_CREATE_ACTION; > - if (pfile_info) { > - memcpy((char *)pfile_info, (char *)&pSMBr->CreationTime, > + *oplock |= CIFS_CREATE_ACTION; > + if (file_info) { > + memcpy((char *)file_info, (char *)&pSMBr->CreationTime, > 36 /* CreationTime to Attributes */); > /* the file_info buf is endian converted by caller */ > - pfile_info->AllocationSize = pSMBr->AllocationSize; > - pfile_info->EndOfFile = pSMBr->EndOfFile; > - pfile_info->NumberOfLinks = cpu_to_le32(1); > - pfile_info->DeletePending = 0; > + file_info->AllocationSize = pSMBr->AllocationSize; > + file_info->EndOfFile = pSMBr->EndOfFile; > + file_info->NumberOfLinks = cpu_to_le32(1); > + file_info->DeletePending = 0; > } > } > > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c > index 8719bbe..6975072 100644 > --- a/fs/cifs/dir.c > +++ b/fs/cifs/dir.c > @@ -203,6 +203,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid, > FILE_ALL_INFO *buf = NULL; > struct inode *newinode = NULL; > int disposition; > + int share_access; > struct TCP_Server_Info *server = tcon->ses->server; > > *oplock = 0; > @@ -293,6 +294,8 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid, > else > cFYI(1, "Create flag not set in create function"); > > + share_access = cifs_get_share_flags(oflags); > + > /* > * BB add processing to set equivalent of mode - e.g. via CreateX with > * ACLs > @@ -320,8 +323,8 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid, > create_options |= CREATE_OPEN_BACKUP_INTENT; > > rc = server->ops->open(xid, tcon, full_path, disposition, > - desired_access, create_options, fid, oplock, > - buf, cifs_sb); > + desired_access, share_access, create_options, > + fid, oplock, buf, cifs_sb); > if (rc) { > cFYI(1, "cifs_create returned 0x%x", rc); > goto out; > @@ -626,9 +629,9 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode, > if (backup_cred(cifs_sb)) > create_options |= CREATE_OPEN_BACKUP_INTENT; > > - rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE, > - GENERIC_WRITE, create_options, > - &fileHandle, &oplock, buf, cifs_sb->local_nls, > + rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_CREATE, GENERIC_WRITE, > + FILE_SHARE_ALL, create_options, &fileHandle, &oplock, > + buf, cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > if (rc) > goto mknod_out; > diff --git a/fs/cifs/file.c b/fs/cifs/file.c > index 8ea6ca5..3ad484c 100644 > --- a/fs/cifs/file.c > +++ b/fs/cifs/file.c > @@ -174,6 +174,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb, > { > int rc; > int desired_access; > + int share_access; > int disposition; > int create_options = CREATE_NOT_DIR; > FILE_ALL_INFO *buf; > @@ -209,6 +210,7 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb, > *********************************************************************/ > > disposition = cifs_get_disposition(f_flags); > + share_access = cifs_get_share_flags(f_flags); > > /* BB pass O_SYNC flag through on file attributes .. BB */ > > @@ -220,8 +222,8 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb, > create_options |= CREATE_OPEN_BACKUP_INTENT; > > rc = server->ops->open(xid, tcon, full_path, disposition, > - desired_access, create_options, fid, oplock, buf, > - cifs_sb); > + desired_access, share_access, create_options, > + fid, oplock, buf, cifs_sb); > > if (rc) > goto out; > @@ -579,6 +581,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush) > struct inode *inode; > char *full_path = NULL; > int desired_access; > + int share_access; > int disposition = FILE_OPEN; > int create_options = CREATE_NOT_DIR; > struct cifs_fid fid; > @@ -643,6 +646,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush) > } > > desired_access = cifs_convert_flags(cfile->f_flags); > + share_access = cifs_get_share_flags(cfile->f_flags); > > if (backup_cred(cifs_sb)) > create_options |= CREATE_OPEN_BACKUP_INTENT; > @@ -658,8 +662,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush) > * not dirty locally we could do this. > */ > rc = server->ops->open(xid, tcon, full_path, disposition, > - desired_access, create_options, &fid, &oplock, > - NULL, cifs_sb); > + desired_access, share_access, create_options, > + &fid, &oplock, NULL, cifs_sb); > if (rc) { > mutex_unlock(&cfile->fh_mutex); > cFYI(1, "cifs_reopen returned 0x%x", rc); > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index ed6208f..a497dfa 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -396,8 +396,8 @@ cifs_sfu_type(struct cifs_fattr *fattr, const unsigned char *path, > tcon = tlink_tcon(tlink); > > rc = CIFSSMBOpen(xid, tcon, path, FILE_OPEN, GENERIC_READ, > - CREATE_NOT_DIR, &netfid, &oplock, NULL, > - cifs_sb->local_nls, > + FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock, > + NULL, cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & > CIFS_MOUNT_MAP_SPECIAL_CHR); > if (rc == 0) { > @@ -987,8 +987,9 @@ cifs_rename_pending_delete(const char *full_path, struct dentry *dentry, > tcon = tlink_tcon(tlink); > > rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, > - DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR, > - &netfid, &oplock, NULL, cifs_sb->local_nls, > + DELETE|FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL, > + CREATE_NOT_DIR, &netfid, &oplock, NULL, > + cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > if (rc != 0) > goto out; > @@ -1509,7 +1510,7 @@ cifs_do_rename(const unsigned int xid, struct dentry *from_dentry, > > /* open the file to be renamed -- we need DELETE perms */ > rc = CIFSSMBOpen(xid, tcon, from_path, FILE_OPEN, DELETE, > - CREATE_NOT_DIR, &srcfid, &oplock, NULL, > + FILE_SHARE_ALL, CREATE_NOT_DIR, &srcfid, &oplock, NULL, > cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & > CIFS_MOUNT_MAP_SPECIAL_CHR); > if (rc == 0) { > diff --git a/fs/cifs/link.c b/fs/cifs/link.c > index 51dc2fb..9b4f0db 100644 > --- a/fs/cifs/link.c > +++ b/fs/cifs/link.c > @@ -212,7 +212,7 @@ CIFSCreateMFSymLink(const unsigned int xid, struct cifs_tcon *tcon, > create_options |= CREATE_OPEN_BACKUP_INTENT; > > rc = CIFSSMBOpen(xid, tcon, fromName, FILE_CREATE, GENERIC_WRITE, > - create_options, &netfid, &oplock, NULL, > + FILE_SHARE_ALL, create_options, &netfid, &oplock, NULL, > nls_codepage, remap); > if (rc != 0) { > kfree(buf); > @@ -254,8 +254,8 @@ CIFSQueryMFSymLink(const unsigned int xid, struct cifs_tcon *tcon, > FILE_ALL_INFO file_info; > > rc = CIFSSMBOpen(xid, tcon, searchName, FILE_OPEN, GENERIC_READ, > - CREATE_NOT_DIR, &netfid, &oplock, &file_info, > - nls_codepage, remap); > + FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock, > + &file_info, nls_codepage, remap); > if (rc != 0) > return rc; > > @@ -332,8 +332,8 @@ CIFSCheckMFSymlink(struct cifs_fattr *fattr, > pTcon = tlink_tcon(tlink); > > rc = CIFSSMBOpen(xid, pTcon, path, FILE_OPEN, GENERIC_READ, > - CREATE_NOT_DIR, &netfid, &oplock, &file_info, > - cifs_sb->local_nls, > + FILE_SHARE_ALL, CREATE_NOT_DIR, &netfid, &oplock, > + &file_info, cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & > CIFS_MOUNT_MAP_SPECIAL_CHR); > if (rc != 0) > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > index cdd6ff4..726b52e 100644 > --- a/fs/cifs/readdir.c > +++ b/fs/cifs/readdir.c > @@ -224,7 +224,7 @@ int get_symlink_reparse_path(char *full_path, struct cifs_sb_info *cifs_sb, > char *tmpbuffer; > > rc = CIFSSMBOpen(xid, ptcon, full_path, FILE_OPEN, GENERIC_READ, > - OPEN_REPARSE_POINT, &fid, &oplock, NULL, > + FILE_SHARE_ALL, OPEN_REPARSE_POINT, &fid, &oplock, NULL, > cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > if (!rc) { > diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c > index 47bc5a8..d782209 100644 > --- a/fs/cifs/smb1ops.c > +++ b/fs/cifs/smb1ops.c > @@ -671,9 +671,9 @@ cifs_mkdir_setinfo(struct inode *inode, const char *full_path, > > static int > cifs_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path, > - int disposition, int desired_access, int create_options, > - struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf, > - struct cifs_sb_info *cifs_sb) > + int disposition, int desired_access, int share_access, > + int create_options, struct cifs_fid *fid, __u32 *oplock, > + FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb) > { > if (!(tcon->ses->capabilities & CAP_NT_SMBS)) > return SMBLegacyOpen(xid, tcon, path, disposition, > @@ -682,8 +682,8 @@ cifs_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path, > cifs_sb->local_nls, cifs_sb->mnt_cifs_flags > & CIFS_MOUNT_MAP_SPECIAL_CHR); > return CIFSSMBOpen(xid, tcon, path, disposition, desired_access, > - create_options, &fid->netfid, oplock, buf, > - cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & > + share_access, create_options, &fid->netfid, oplock, > + buf, cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & > CIFS_MOUNT_MAP_SPECIAL_CHR); > } > > @@ -779,8 +779,9 @@ smb_set_file_info(struct inode *inode, const char *full_path, > cFYI(1, "calling SetFileInfo since SetPathInfo for times not supported " > "by this server"); > rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, > - SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR, > - &netfid, &oplock, NULL, cifs_sb->local_nls, > + SYNCHRONIZE | FILE_WRITE_ATTRIBUTES, FILE_SHARE_ALL, > + CREATE_NOT_DIR, &netfid, &oplock, NULL, > + cifs_sb->local_nls, > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > > if (rc != 0) { > diff --git a/fs/cifs/smb2file.c b/fs/cifs/smb2file.c > index 71e6aed..7dfb50c 100644 > --- a/fs/cifs/smb2file.c > +++ b/fs/cifs/smb2file.c > @@ -58,9 +58,9 @@ smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock) > > int > smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path, > - int disposition, int desired_access, int create_options, > - struct cifs_fid *fid, __u32 *oplock, FILE_ALL_INFO *buf, > - struct cifs_sb_info *cifs_sb) > + int disposition, int desired_access, int share_access, > + int create_options, struct cifs_fid *fid, __u32 *oplock, > + FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb) > { > int rc; > __le16 *smb2_path; > @@ -87,8 +87,8 @@ smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, const char *path, > memcpy(smb2_oplock + 1, fid->lease_key, SMB2_LEASE_KEY_SIZE); > > rc = SMB2_open(xid, tcon, smb2_path, &fid->persistent_fid, > - &fid->volatile_fid, desired_access, disposition, > - 0, 0, smb2_oplock, smb2_data); > + &fid->volatile_fid, desired_access, share_access, > + disposition, 0, 0, smb2_oplock, smb2_data); > if (rc) > goto out; > > diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c > index 7064824..6af174a 100644 > --- a/fs/cifs/smb2inode.c > +++ b/fs/cifs/smb2inode.c > @@ -54,8 +54,8 @@ smb2_open_op_close(const unsigned int xid, struct cifs_tcon *tcon, > return -ENOMEM; > > rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid, > - desired_access, create_disposition, file_attributes, > - create_options, &oplock, NULL); > + desired_access, FILE_SHARE_ALL, create_disposition, > + file_attributes, create_options, &oplock, NULL); > if (rc) { > kfree(utf16_path); > return rc; > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index c9c7aa7..dc38434 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -222,7 +222,8 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, > return -ENOMEM; > > rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid, > - FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL); > + FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0, > + &oplock, NULL); > if (rc) { > kfree(utf16_path); > return rc; > @@ -432,8 +433,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon, > return -ENOMEM; > > rc = SMB2_open(xid, tcon, utf16_path, &persistent_fid, &volatile_fid, > - FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_OPEN, 0, 0, > - &oplock, NULL); > + FILE_READ_ATTRIBUTES | FILE_READ_DATA, FILE_SHARE_ALL, > + FILE_OPEN, 0, 0, &oplock, NULL); > kfree(utf16_path); > if (rc) { > cERROR(1, "open dir failed"); > @@ -515,7 +516,8 @@ smb2_queryfs(const unsigned int xid, struct cifs_tcon *tcon, > u8 oplock = SMB2_OPLOCK_LEVEL_NONE; > > rc = SMB2_open(xid, tcon, &srch_path, &persistent_fid, &volatile_fid, > - FILE_READ_ATTRIBUTES, FILE_OPEN, 0, 0, &oplock, NULL); > + FILE_READ_ATTRIBUTES, FILE_SHARE_ALL, FILE_OPEN, 0, 0, > + &oplock, NULL); > if (rc) > return rc; > buf->f_type = SMB2_MAGIC_NUMBER; > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 41d9d07..45fa2dc 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -910,8 +910,8 @@ parse_lease_state(struct smb2_create_rsp *rsp) > int > SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, __le16 *path, > u64 *persistent_fid, u64 *volatile_fid, __u32 desired_access, > - __u32 create_disposition, __u32 file_attributes, __u32 create_options, > - __u8 *oplock, struct smb2_file_all_info *buf) > + __u32 share_access, __u32 create_disposition, __u32 file_attributes, > + __u32 create_options, __u8 *oplock, struct smb2_file_all_info *buf) > { > struct smb2_create_req *req; > struct smb2_create_rsp *rsp; > @@ -940,7 +940,7 @@ SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, __le16 *path, > req->DesiredAccess = cpu_to_le32(desired_access); > /* File attributes ignored on open (used in create though) */ > req->FileAttributes = cpu_to_le32(file_attributes); > - req->ShareAccess = FILE_SHARE_ALL_LE; > + req->ShareAccess = cpu_to_le32(share_access); > req->CreateDisposition = cpu_to_le32(create_disposition); > req->CreateOptions = cpu_to_le32(create_options); > uni_path_len = (2 * UniStrnlen((wchar_t *)path, PATH_MAX)) + 2; > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h > index 2aa3535..edff8f6 100644 > --- a/fs/cifs/smb2proto.h > +++ b/fs/cifs/smb2proto.h > @@ -86,9 +86,10 @@ extern int smb2_create_hardlink(const unsigned int xid, struct cifs_tcon *tcon, > > extern int smb2_open_file(const unsigned int xid, struct cifs_tcon *tcon, > const char *full_path, int disposition, > - int desired_access, int create_options, > - struct cifs_fid *fid, __u32 *oplock, > - FILE_ALL_INFO *buf, struct cifs_sb_info *cifs_sb); > + int desired_access, int share_access, > + int create_options, struct cifs_fid *fid, > + __u32 *oplock, FILE_ALL_INFO *buf, > + struct cifs_sb_info *cifs_sb); > extern void smb2_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock); > extern int smb2_unlock_range(struct cifsFileInfo *cfile, > struct file_lock *flock, const unsigned int xid); > @@ -108,9 +109,10 @@ extern int SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, > extern int SMB2_tdis(const unsigned int xid, struct cifs_tcon *tcon); > extern int SMB2_open(const unsigned int xid, struct cifs_tcon *tcon, > __le16 *path, u64 *persistent_fid, u64 *volatile_fid, > - __u32 desired_access, __u32 create_disposition, > - __u32 file_attributes, __u32 create_options, > - __u8 *oplock, struct smb2_file_all_info *buf); > + __u32 desired_access, __u32 share_access, > + __u32 create_disposition, __u32 file_attributes, > + __u32 create_options, __u8 *oplock, > + struct smb2_file_all_info *buf); > extern int SMB2_close(const unsigned int xid, struct cifs_tcon *tcon, > u64 persistent_file_id, u64 volatile_file_id); > extern int SMB2_flush(const unsigned int xid, struct cifs_tcon *tcon, -- Jeff Layton <jlayton(a)poochiereds.net>

March 11, 2013

Re: [PATCH v3 2/7] vfs: Add O_DENYREAD/WRITE flags support for open syscall

by Jeff Layton

On Thu, 28 Feb 2013 19:25:28 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > If O_DENYMAND flag is specified, O_DENYREAD/WRITE/MAND flags are > translated to flock's flags: > > !O_DENYREAD -> LOCK_READ > !O_DENYWRITE -> LOCK_WRITE > O_DENYMAND -> LOCK_MAND > > and set through flock_lock_file on a file. > > This change affects opens that use O_DENYMAND flag - all other > native Linux opens don't care about these flags. It allow us to > enable this feature for applications that need it (e.g. NFS and > Samba servers that export the same directory for Windows clients, > or Wine applications that access the same files simultaneously). > > Create codepath is slightly changed to prevent data races on > newely created files: when open with O_CREAT can return with -ETXTBSY > error for successfully created files due to a deny lock set by > another task. > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > --- > fs/locks.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++------ > fs/namei.c | 44 ++++++++++++++++++-- > include/linux/fs.h | 6 +++ > 3 files changed, 151 insertions(+), 15 deletions(-) > > diff --git a/fs/locks.c b/fs/locks.c > index a94e331..0cc7d1b 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -605,20 +605,81 @@ static int posix_locks_conflict(struct file_lock *caller_fl, struct file_lock *s > return (locks_conflict(caller_fl, sys_fl)); > } > > -/* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific > - * checking before calling the locks_conflict(). > +static unsigned int > +deny_flags_to_cmd(unsigned int flags) > +{ > + unsigned int cmd = LOCK_MAND; > + > + if (!(flags & O_DENYREAD)) > + cmd |= LOCK_READ; > + if (!(flags & O_DENYWRITE)) > + cmd |= LOCK_WRITE; > + > + return cmd; > +} > + > +/* > + * locks_mand_conflict - Determine if there's a share reservation conflict > + * @caller_fl: lock we're attempting to acquire > + * @sys_fl: lock already present on system that we're checking against > + * > + * Check to see if there's a share_reservation conflict. LOCK_READ/LOCK_WRITE > + * tell us whether the reservation allows other readers and writers. > + * > + * We only check against other LOCK_MAND locks, so applications that want to > + * use share mode locking will only conflict against one another. "normal" > + * applications that open files won't be affected by and won't themselves > + * affect the share reservations. > */ > -static int flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) > +static int > +locks_mand_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl) > { > - /* FLOCK locks referring to the same filp do not conflict with > + unsigned char caller_type = caller_fl->fl_type; > + unsigned char sys_type = sys_fl->fl_type; > + fmode_t caller_fmode = caller_fl->fl_file->f_mode; > + fmode_t sys_fmode = sys_fl->fl_file->f_mode; > + > + /* they can only conflict if they're both LOCK_MAND */ > + if (!(caller_type & LOCK_MAND) || !(sys_type & LOCK_MAND)) > + return 0; > + > + if (!(caller_type & LOCK_READ) && (sys_fmode & FMODE_READ)) > + return 1; > + if (!(caller_type & LOCK_WRITE) && (sys_fmode & FMODE_WRITE)) > + return 1; > + if (!(sys_type & LOCK_READ) && (caller_fmode & FMODE_READ)) > + return 1; > + if (!(sys_type & LOCK_WRITE) && (caller_fmode & FMODE_WRITE)) > + return 1; > + > + return 0; > +} > + > +/* > + * Determine if lock sys_fl blocks lock caller_fl. FLOCK specific checking > + * before calling the locks_conflict(). Boolean is_mand indicates whether > + * we should use a share reservation scheme or not. > + */ > +static int > +flock_locks_conflict(struct file_lock *caller_fl, struct file_lock *sys_fl, > + bool is_mand) I'm not sure you really need to add this new is_mand bool. Won't that be equivalent to (caller_fl->fl_type & LOCK_MAND)? > +{ > + /* > + * FLOCK locks referring to the same filp do not conflict with > * each other. > */ > - if (!IS_FLOCK(sys_fl) || (caller_fl->fl_file == sys_fl->fl_file)) > - return (0); > - if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND)) > + if (!IS_FLOCK(sys_fl)) > + return 0; > + if ((caller_fl->fl_type & LOCK_MAND) || (sys_fl->fl_type & LOCK_MAND)) { > + if (is_mand) > + return locks_mand_conflict(caller_fl, sys_fl); > + else > + return 0; > + } > + if (caller_fl->fl_file == sys_fl->fl_file) > return 0; > > - return (locks_conflict(caller_fl, sys_fl)); > + return locks_conflict(caller_fl, sys_fl); > } > > void > @@ -697,14 +758,19 @@ static int posix_locks_deadlock(struct file_lock *caller_fl, > return 0; > } > > -/* Try to create a FLOCK lock on filp. We always insert new FLOCK locks > +/* > + * Try to create a FLOCK lock on filp. We always insert new FLOCK locks > * after any leases, but before any posix locks. > * > * Note that if called with an FL_EXISTS argument, the caller may determine > * whether or not a lock was successfully freed by testing the return > * value for -ENOENT. > + * > + * Take @is_conflict callback that determines how to check if locks have > + * conflicts or not. > */ > -static int flock_lock_file(struct file *filp, struct file_lock *request) > +static int > +flock_lock_file(struct file *filp, struct file_lock *request, bool is_mand) Ditto here on the is_mand bool. I think you can determine that from request->fl_type. Right? > { > struct file_lock *new_fl = NULL; > struct file_lock **before; > @@ -760,7 +826,7 @@ find_conflict: > break; > if (IS_LEASE(fl)) > continue; > - if (!flock_locks_conflict(request, fl)) > + if (!flock_locks_conflict(request, fl, is_mand)) > continue; > error = -EAGAIN; > if (!(request->fl_flags & FL_SLEEP)) > @@ -783,6 +849,32 @@ out: > return error; > } > > +/* > + * Determine if a file is allowed to be opened with specified access and deny > + * modes. Lock the file and return 0 if checks passed, otherwise return a error > + * code. > + */ > +int > +deny_lock_file(struct file *filp) > +{ > + struct file_lock *lock; > + int error = 0; > + > + if (!(filp->f_flags & O_DENYMAND)) > + return error; > + > + error = flock_make_lock(filp, &lock, deny_flags_to_cmd(filp->f_flags)); > + if (error) > + return error; > + > + error = flock_lock_file(filp, lock, true); > + if (error == -EAGAIN) > + error = -ETXTBSY; > + I think EBUSY would be a better return code here. ETXTBSY is returned in more specific circumstances -- mostly when you're opening a file for write that is being executed. > + locks_free_lock(lock); > + return error; > +} > + > static int __posix_lock_file(struct inode *inode, struct file_lock *request, struct file_lock *conflock) > { > struct file_lock *fl; > @@ -1589,7 +1681,7 @@ int flock_lock_file_wait(struct file *filp, struct file_lock *fl) > int error; > might_sleep(); > for (;;) { > - error = flock_lock_file(filp, fl); > + error = flock_lock_file(filp, fl, false); > if (error != FILE_LOCK_DEFERRED) > break; > error = wait_event_interruptible(fl->fl_wait, !fl->fl_next); > diff --git a/fs/namei.c b/fs/namei.c > index 43a97ee..c1f7e08 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -2559,9 +2559,14 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry, > * here. > */ > error = may_open(&file->f_path, acc_mode, open_flag); > - if (error) > + if (error) { > fput(file); > + goto out; > + } > > + error = deny_lock_file(file); > + if (error) > + fput(file); > out: > dput(dentry); > return error; > @@ -2771,9 +2776,9 @@ retry_lookup: > } > mutex_lock(&dir->d_inode->i_mutex); > error = lookup_open(nd, path, file, op, got_write, opened); > - mutex_unlock(&dir->d_inode->i_mutex); > > if (error <= 0) { > + mutex_unlock(&dir->d_inode->i_mutex); > if (error) > goto out; > > @@ -2791,8 +2796,32 @@ retry_lookup: > will_truncate = false; > acc_mode = MAY_OPEN; > path_to_nameidata(path, nd); > - goto finish_open_created; > + > + /* > + * Unlock parent i_mutex later when the open finishes - prevent > + * races when a file can be locked with a deny lock by another > + * task that opens the file. > + */ > + error = may_open(&nd->path, acc_mode, open_flag); > + if (error) { > + mutex_unlock(&dir->d_inode->i_mutex); > + goto out; > + } > + file->f_path.mnt = nd->path.mnt; > + error = finish_open(file, nd->path.dentry, NULL, opened); > + if (error) { > + mutex_unlock(&dir->d_inode->i_mutex); > + if (error == -EOPENSTALE) > + goto stale_open; > + goto out; > + } > + error = deny_lock_file(file); > + mutex_unlock(&dir->d_inode->i_mutex); > + if (error) > + goto exit_fput; > + goto opened; > } > + mutex_unlock(&dir->d_inode->i_mutex); > > /* > * create/update audit record if it already exists. > @@ -2885,6 +2914,15 @@ finish_open_created: > goto stale_open; > goto out; > } > + /* > + * Lock parent i_mutex to prevent races with deny locks on newely > + * created files. > + */ > + mutex_lock(&dir->d_inode->i_mutex); > + error = deny_lock_file(file); > + mutex_unlock(&dir->d_inode->i_mutex); > + if (error) > + goto exit_fput; > opened: > error = open_check_o_direct(file); > if (error) > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 7617ee0..347e1de 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1005,6 +1005,7 @@ extern int lease_modify(struct file_lock **, int); > extern int lock_may_read(struct inode *, loff_t start, unsigned long count); > extern int lock_may_write(struct inode *, loff_t start, unsigned long count); > extern void locks_delete_block(struct file_lock *waiter); > +extern int deny_lock_file(struct file *); > extern void lock_flocks(void); > extern void unlock_flocks(void); > #else /* !CONFIG_FILE_LOCKING */ > @@ -1153,6 +1154,11 @@ static inline void locks_delete_block(struct file_lock *waiter) > { > } > > +static inline int deny_lock_file(struct file *filp) > +{ > + return 0; > +} > + > static inline void lock_flocks(void) > { > } -- Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: [PATCH v3 5/7] CIFS: Translate SHARING_VIOLATION to -ETXTBSY error code for SMB2

by Jeff Layton

On Thu, 28 Feb 2013 19:25:31 +0400 Pavel Shilovsky <piastry(a)etersoft.ru> wrote: > to make it match CIFS and VFS variants. > > Signed-off-by: Pavel Shilovsky <piastry(a)etersoft.ru> > --- > fs/cifs/smb2maperror.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2maperror.c b/fs/cifs/smb2maperror.c > index 494c912..11e589e 100644 > --- a/fs/cifs/smb2maperror.c > +++ b/fs/cifs/smb2maperror.c > @@ -356,7 +356,7 @@ static const struct status_to_posix_error smb2_error_map_table[] = { > {STATUS_PORT_CONNECTION_REFUSED, -ECONNREFUSED, > "STATUS_PORT_CONNECTION_REFUSED"}, > {STATUS_INVALID_PORT_HANDLE, -EIO, "STATUS_INVALID_PORT_HANDLE"}, > - {STATUS_SHARING_VIOLATION, -EBUSY, "STATUS_SHARING_VIOLATION"}, > + {STATUS_SHARING_VIOLATION, -ETXTBSY, "STATUS_SHARING_VIOLATION"}, > {STATUS_QUOTA_EXCEEDED, -EDQUOT, "STATUS_QUOTA_EXCEEDED"}, > {STATUS_INVALID_PAGE_PROTECTION, -EIO, > "STATUS_INVALID_PAGE_PROTECTION"}, Actually, I think Sachin is converting the CIFS STATUS_SHARING_VIOLATION to translate to EBUSY, since that seems to better reflect the situation. I'd suggest dropping this patch, unless you have a specific need for this error return here. -- Jeff Layton <jlayton(a)redhat.com>

March 11, 2013

Re: [PATCH v3 0/7] Add O_DENY* support for VFS and CIFS/NFS

by J. Bruce Fields

On Mon, Mar 11, 2013 at 11:18:15AM -0700, Andy Lutomirski wrote: > On Tue, Mar 5, 2013 at 11:07 AM, Simo <simo(a)samba.org> wrote: > > On 03/05/2013 01:13 PM, J. Bruce Fields wrote: > >> > >> On Mon, Mar 04, 2013 at 05:49:46PM -0500, Simo wrote: > >>> > >>> On 03/04/2013 04:19 PM, J. Bruce Fields wrote: > >>>> > >>>> On Thu, Feb 28, 2013 at 01:53:25PM -0800, Andy Lutomirski wrote: > >>>>> > >>>>> [possible resend -- sorry] > >>>>> > >>>>> On 02/28/2013 07:25 AM, Pavel Shilovsky wrote: > >>>>>> > >>>>>> This patchset adds support of O_DENY* flags for Linux fs layer. These > >>>>>> flags can be used by any application that needs share reservations to > >>>>>> organize a file access. VFS already has some sort of this capability - now > >>>>>> it's done through flock/LOCK_MAND mechanis, but that approach is non-atomic. > >>>>>> This patchset build new capabilities on top of the existing one but doesn't > >>>>>> bring any changes into the flock call semantic. > >>>>>> > >>>>>> These flags can be used by NFS (built-in-kernel) and CIFS (Samba) > >>>>>> servers and Wine applications through VFS (for local filesystems) or > >>>>>> CIFS/NFS modules. This will help when e.g. Samba and NFS server share the > >>>>>> same directory for Windows and Linux users or Wine applications use > >>>>>> Samba/NFS share to access the same data from different clients. > >>>>>> > >>>>>> According to the previous discussions the most problematic question is > >>>>>> how to prevent situations like DoS attacks where e.g /lib/liba.so file can > >>>>>> be open with DENYREAD, or smth like this. That's why one extra flag > >>>>>> O_DENYMAND is added. It indicates to underlying layer that an application > >>>>>> want to use O_DENY* flags semantic. It allows us not affect native Linux > >>>>>> applications (that don't use O_DENYMAND flag) - so, these flags (and the > >>>>>> semantic of open syscall that they bring) are used only for those > >>>>>> applications that really want it proccessed that way. > >>>>>> > >>>>>> So, we have four new flags: > >>>>>> O_DENYREAD - to prevent other opens with read access, > >>>>>> O_DENYWRITE - to prevent other opens with write access, > >>>>>> O_DENYDELETE - to prevent delete operations (this flag is not > >>>>>> implemented in VFS and NFS part and only suitable for CIFS module), > >>>>>> O_DENYMAND - to switch on/off three flags above. > >>>>> > >>>>> O_DENYMAND doesn't deny anything. Would a name like O_RESPECT_DENY be > >>>>> better? > >>>>> > >>>>> Other than that, this seems like a sensible mechanism. > >>>> > >>>> I'm a little more worried: these are mandatory locks, and applications > >>>> that use them are used to the locks being enforced correctly. Are we > >>>> sure that an application that opens a file O_DENYWRITE won't crash if it > >>>> sees the file data change while it holds the open? > >>> > >>> The redirector may simply assume it has full control of that part of > >>> the file and not read nor send data until the lock is released too, > >>> so you get conflicting views of the file contents between different > >>> clients if you let a mandatory lock not be mandatory. > >>> > >>>> In general the idea of making a mandatory lock opt-in makes me nervous. > >>>> I'd prefer something like a mount option, so that we know that everyone > >>>> on that one filesystem is playing by the same rules, but we can still > >>>> mount filesystems like / without the option. > >>> > >>> +1 > >>> > >>>> But I'll admit I'm definitely not an expert on Windows locking and may > >>>> be missing something about how these locks are meant to work. > >>> > >>> Mandatory locks really are mandatory in Windows. > >>> That may not be nice to a Unix system but what can you do ? > >> > >> I wonder if we could repurpose the existing -omand mount option? > >> > >> That would be a problem for anyone that wants to allow mandatory fcntl > >> locks without allowing share locks. I doubt anyone sane actually uses > >> mandatory fcntl locks, but still I suppose it would probably be better > >> to play it safe and use a new mount option. > > > > > > Maybe we should have a -o win_semantics option :-) > > > > It's not entirely obvious to me that allowing programs to bypass this > kind of locking is a bad idea. It's hard to do on Windows, but > presumably network filesystems on Windows already have this issue. Could be, but I'd like to see evidence of that. --b.

March 11, 2013

Re: [PATCH v3 0/7] Add O_DENY* support for VFS and CIFS/NFS

by Andy Lutomirski

On Tue, Mar 5, 2013 at 11:07 AM, Simo <simo(a)samba.org> wrote: > On 03/05/2013 01:13 PM, J. Bruce Fields wrote: >> >> On Mon, Mar 04, 2013 at 05:49:46PM -0500, Simo wrote: >>> >>> On 03/04/2013 04:19 PM, J. Bruce Fields wrote: >>>> >>>> On Thu, Feb 28, 2013 at 01:53:25PM -0800, Andy Lutomirski wrote: >>>>> >>>>> [possible resend -- sorry] >>>>> >>>>> On 02/28/2013 07:25 AM, Pavel Shilovsky wrote: >>>>>> >>>>>> This patchset adds support of O_DENY* flags for Linux fs layer. These >>>>>> flags can be used by any application that needs share reservations to >>>>>> organize a file access. VFS already has some sort of this capability - now >>>>>> it's done through flock/LOCK_MAND mechanis, but that approach is non-atomic. >>>>>> This patchset build new capabilities on top of the existing one but doesn't >>>>>> bring any changes into the flock call semantic. >>>>>> >>>>>> These flags can be used by NFS (built-in-kernel) and CIFS (Samba) >>>>>> servers and Wine applications through VFS (for local filesystems) or >>>>>> CIFS/NFS modules. This will help when e.g. Samba and NFS server share the >>>>>> same directory for Windows and Linux users or Wine applications use >>>>>> Samba/NFS share to access the same data from different clients. >>>>>> >>>>>> According to the previous discussions the most problematic question is >>>>>> how to prevent situations like DoS attacks where e.g /lib/liba.so file can >>>>>> be open with DENYREAD, or smth like this. That's why one extra flag >>>>>> O_DENYMAND is added. It indicates to underlying layer that an application >>>>>> want to use O_DENY* flags semantic. It allows us not affect native Linux >>>>>> applications (that don't use O_DENYMAND flag) - so, these flags (and the >>>>>> semantic of open syscall that they bring) are used only for those >>>>>> applications that really want it proccessed that way. >>>>>> >>>>>> So, we have four new flags: >>>>>> O_DENYREAD - to prevent other opens with read access, >>>>>> O_DENYWRITE - to prevent other opens with write access, >>>>>> O_DENYDELETE - to prevent delete operations (this flag is not >>>>>> implemented in VFS and NFS part and only suitable for CIFS module), >>>>>> O_DENYMAND - to switch on/off three flags above. >>>>> >>>>> O_DENYMAND doesn't deny anything. Would a name like O_RESPECT_DENY be >>>>> better? >>>>> >>>>> Other than that, this seems like a sensible mechanism. >>>> >>>> I'm a little more worried: these are mandatory locks, and applications >>>> that use them are used to the locks being enforced correctly. Are we >>>> sure that an application that opens a file O_DENYWRITE won't crash if it >>>> sees the file data change while it holds the open? >>> >>> The redirector may simply assume it has full control of that part of >>> the file and not read nor send data until the lock is released too, >>> so you get conflicting views of the file contents between different >>> clients if you let a mandatory lock not be mandatory. >>> >>>> In general the idea of making a mandatory lock opt-in makes me nervous. >>>> I'd prefer something like a mount option, so that we know that everyone >>>> on that one filesystem is playing by the same rules, but we can still >>>> mount filesystems like / without the option. >>> >>> +1 >>> >>>> But I'll admit I'm definitely not an expert on Windows locking and may >>>> be missing something about how these locks are meant to work. >>> >>> Mandatory locks really are mandatory in Windows. >>> That may not be nice to a Unix system but what can you do ? >> >> I wonder if we could repurpose the existing -omand mount option? >> >> That would be a problem for anyone that wants to allow mandatory fcntl >> locks without allowing share locks. I doubt anyone sane actually uses >> mandatory fcntl locks, but still I suppose it would probably be better >> to play it safe and use a new mount option. > > > Maybe we should have a -o win_semantics option :-) > It's not entirely obvious to me that allowing programs to bypass this kind of locking is a bad idea. It's hard to do on Windows, but presumably network filesystems on Windows already have this issue. --Andy

March 11, 2013

Re: [PATCH v3 0/7] Add O_DENY* support for VFS and CIFS/NFS

by Pavel Shilovsky

2013/3/5 Simo <simo(a)samba.org>: > On 03/05/2013 01:13 PM, J. Bruce Fields wrote: >> >> On Mon, Mar 04, 2013 at 05:49:46PM -0500, Simo wrote: >>> >>> On 03/04/2013 04:19 PM, J. Bruce Fields wrote: >>>> I'm a little more worried: these are mandatory locks, and applications >>>> that use them are used to the locks being enforced correctly. Are we >>>> sure that an application that opens a file O_DENYWRITE won't crash if it >>>> sees the file data change while it holds the open? >>> >>> The redirector may simply assume it has full control of that part of >>> the file and not read nor send data until the lock is released too, >>> so you get conflicting views of the file contents between different >>> clients if you let a mandatory lock not be mandatory. >>> >>>> In general the idea of making a mandatory lock opt-in makes me nervous. >>>> I'd prefer something like a mount option, so that we know that everyone >>>> on that one filesystem is playing by the same rules, but we can still >>>> mount filesystems like / without the option. >>> >>> +1 >>> >>>> But I'll admit I'm definitely not an expert on Windows locking and may >>>> be missing something about how these locks are meant to work. >>> >>> Mandatory locks really are mandatory in Windows. >>> That may not be nice to a Unix system but what can you do ? >> >> I wonder if we could repurpose the existing -omand mount option? >> >> That would be a problem for anyone that wants to allow mandatory fcntl >> locks without allowing share locks. I doubt anyone sane actually uses >> mandatory fcntl locks, but still I suppose it would probably be better >> to play it safe and use a new mount option. > > > Maybe we should have a -o win_semantics option :-) > > /me runs > (CC'ing Al Viro, since these patches should go through his tree) I don't mind to introduce a new mount option for turning this feature on/off - something like '-o denylock' to make it mathing names of new flags would be ok. Al, what do you think about this feature overall? -- Best regards, Pavel Shilovsky.

March 11, 2013

Re: ntdll: make NtDelayExecution a bit more efficient

by Graham Knap

Francois Gouget wrote: > I think it's really a three patch series and the first one has the > clearer commit message: > > Made NtDelayExecution with a 0 timeout yield the CPU, as it is > supposed to. I agree that this is the correct behaviour for the "zero timeout" case. The comments that I submitted with my patch were incorrect, because I misread the existing code. The patch that I submitted would not have changed this behaviour, though it did reorganize a few lines of code. I had proposed to change the "nonzero timeout" case. If the caller specifies a non-zero timeout value, the existing code calls: 1 if the timeout is negative: NtQuerySystemTime 2 NtYieldExecution 3 begin loop: 3.1 NtQuerySystemTime 3.2 select We break out of the loop when "select" has waited the specified time, and was not interrupted by a signal. (Right? Please tell me if I've misinterpreted the code again.) I suggested changing it to: 1 NtQuerySystemTime 2 begin loop: 2.1 select 2.2 NtQuerySystemTime To explain: * I think there is no need to call sched_yield before blocking on select. (But I haven't yet come up with a way to prove it.) * We no longer need to measure how long sched_yield made us wait, so we can reorder the NtQuerySystemTime calls. The break condition is unchanged, so the second NtQuerySystemTime call is often skipped. > http://www.winehq.org/pipermail/wine-devel/2004-November/030796.html Thanks, I will look at this. -- graham

March 11, 2013

Re: [2/2] wmvcore: Stub implementation of IWMMetadataEditor interface (try2)

by Nikolay Sivov

On 3/11/2013 16:36, Jeff Latimer wrote: > + if (!ppv) return E_INVALIDARG; You don't need this most likely. > + IUnknown_AddRef((IUnknown*)*ppv); A cleaner way is to AddRef original iface, but that's a matter of taste. > +static ULONG WINAPI WMCreateEditor_AddRef(IWMMetadataEditor *iface) > +{ > + FIXME("iface=%p\n", iface); > + return 2; > +} > + > +static ULONG WINAPI WMCreateEditor_Release(IWMMetadataEditor *iface) > +{ > + FIXME("iface=%p\n", iface); > + HeapFree(GetProcessHeap(), 0, iface); > + return 1; > +} What's this supposed to mean? > +HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface) > +{ > + FIXME("iface=%p\n", iface); > + HeapFree(GetProcessHeap(), 0, iface); > + return S_OK; > +} > + > +HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface) > +{ > + FIXME("iface=%p\n", iface); > + HeapFree(GetProcessHeap(), 0, iface); > + return S_OK; > +} I don't think so. > HRESULT WINAPI WMCreateEditor(IWMMetadataEditor **editor) > { > + IWMMetadataEditor *This; > FIXME("(%p): stub\n", editor); > > - *editor = NULL; > + This = HeapAlloc(GetProcessHeap(), 0, sizeof(IWMMetadataEditor)); > + if (!This) return E_OUTOFMEMORY; > > - return E_NOTIMPL; > + This->lpVtbl = &Editor_vtable; > + *editor = This; > + > + return S_OK; > } That's not how it's done, there's plenty of examples already. > -}; > +} That change is ok I think.

March 11, 2013

Re: [1/2] include: Implement part of nserror.h

by Nikolay Sivov

On 3/11/2013 13:41, Jeff Latimer wrote: > On 11/03/13 17:43, Nikolay Sivov wrote: >> On 3/11/2013 10:27, Jeff Latimer wrote: >>> --- >>> include/nserror.h | 226 >>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 226 insertions(+) >>> create mode 100644 include/nserror.h >>> +#ifndef _NSERROR_H >>> +#define _NSERROR_H >> This doesn't look right. > This is the same as the SDK, atleast to my eye. Is there something in > particular that appears wrong? See how it's done in existing headers. >> >>> +#define NS_S_CALLPENDING _HRESULT_TYPEDEF_(0x000D0000L) >> This is usually done with a suffix. > I have done this as the SDK does it. I thought that was the way we > liked it. Sorry, I meant to say 'without a suffix'. See how _HRESULT_TYPEDEF_() is used in winerror.h for example. >> >>> + >>> +#endif >> Please add a comment here. > Ok.

March 11, 2013

Re: [1/2] include: Implement part of nserror.h

by Jeff Latimer

On 11/03/13 17:43, Nikolay Sivov wrote: > On 3/11/2013 10:27, Jeff Latimer wrote: >> --- >> include/nserror.h | 226 >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 226 insertions(+) >> create mode 100644 include/nserror.h >> +#ifndef _NSERROR_H >> +#define _NSERROR_H > This doesn't look right. This is the same as the SDK, atleast to my eye. Is there something in particular that appears wrong? > >> +#define NS_S_CALLPENDING _HRESULT_TYPEDEF_(0x000D0000L) > This is usually done with a suffix. I have done this as the SDK does it. I thought that was the way we liked it. > >> + >> +#endif > Please add a comment here. Ok.

March 11, 2013

Re: [1/2] include: Implement part of nserror.h

by Nikolay Sivov

On 3/11/2013 10:27, Jeff Latimer wrote: > --- > include/nserror.h | 226 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 226 insertions(+) > create mode 100644 include/nserror.h > +#ifndef _NSERROR_H > +#define _NSERROR_H This doesn't look right. > +#define NS_S_CALLPENDING_HRESULT_TYPEDEF_(0x000D0000L) This is usually done with a suffix. > + > +#endif Please add a comment here.

March 11, 2013

Re: msvcp60: Avoid signed-unsigned integer comparisons

by Max TenEyck Woodbury

On 03/10/2013 07:00 AM, Henri Verbeet wrote: > On 10 March 2013 08:20, larmbr zhan <nasa4836(a)gmail.com> wrote: >> >> But It behaves different when it is signed or not. According to >> C Standard, >> >> - For the signed case, once it overflows, resulting in >> representing a negative value . > > Actually, signed overflow behavior is undefined according to the > standard. Specifically, some hardware throws an exception on signed arithmetic overflow and C is specifically designed to be hardware independent, so it has to be 'undefined behavior'. >> - For the unsigned case, once it overflows, resulting in >> representing a value reduced modulo the largest value >> that object could hold. Nit: modulo the largest value the object can hold _plus one_, but it should be treated as another 'undefined behavior'. To further complicate the issue, while size_t is always unsigned, size_t can be 16, 18, 32, 36 or 64 bits and still be compliant. So use 'size_t' when you are talking about the size of something, and 'unsigned' for flag sets and counts.

March 10, 2013

Re: msvcp60: Avoid signed-unsigned integer comparisons

by Henri Verbeet

On 10 March 2013 08:20, larmbr zhan <nasa4836(a)gmail.com> wrote: > But It behaves different when it is signed or not. According to C Standard, > - For the signed case, once it overflows, resulting in > representing a negative value . Actually, signed overflow behaviour is undefined according to the standard.

March 10, 2013

Re: msvcp60: Avoid signed-unsigned integer comparisons

by larmbr zhan

On Sun, Mar 10, 2013 at 5:48 AM, Andrew Talbot <andrew.talbot(a)talbotville.com> wrote: > msvcp60: Avoid signed-unsigned integer comparisons. Hi, Andrew Talbot. I find that you are working on these "Avoid signed-unsigned integer comparisons" things recently. I DO agree on that using _ unsigned int _ instead of _ int _(which implies _ signed int _). But I think using size_t maybe more standard-compliant, more efficient, and less bug-prone, . To hold this, I think something has to be clarified... * The overflow behavior is instrinsic because the finite range that a object of INT type can hold. But It behaves different when it is signed or not. According to C Standard, - For the signed case, once it overflows, resulting in representing a negative value . - For the unsigned case, once it overflows, resulting in representing a value reduced modulo the largest value that object could hold. So see this example: --- snip --- size_t num = get_max_object_num(); // which may give a large number. unsigned int i; for ( i = 0; i < num; i++) something_important[i] = vital_evaluate_for(i); --- snip --- For this case, though we use unsinged int ,it still may issue problem if num >= UINT_MAX, which will cause i to wrap to 0, then something_important maybe assigned a invalid value, as like. This problem may occur if using in a 64-bit enviroment, where size_t is typeof 'ed unsigned 64bit while unsigned int is still 32-t wide. Some may argue that in Wine this problem will NEVER happen. I don't doubt it. But * since we have a cleared and more standard-compliant way (which means less bug-prone), and * predictably Wine will support more platforms, portable way shall be taken it account. So I think we should use size_t in this case. -- Cheers, Zhan JIanyu

March 10, 2013

Re: wininet: Skip test_open_url_async on IEs older than 7.

by Marvin

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=24685 Your paranoid android. === WVISTAADM (32 bit http) === Failure running script in VM: The specified guest user must be logged in interactively to perform this operation

March 9, 2013

Re: dpnet/tests: Allow buffersize used by Win8

by Marvin

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=24682 Your paranoid android. === WNT4WSSP6 (32 bit peer) === peer.c:52: Test failed: CoCreateInstance failed with 0x80040154 peer: unhandled exception c0000005 at 004013BF === W2KPROSP4 (32 bit peer) === peer.c:52: Test failed: CoCreateInstance failed with 0x80040154 peer: unhandled exception c0000005 at 004013BF

March 9, 2013

Re: Crash confuses me

by Graham

On 09/03/13 14:08, André Hentschel wrote: > Am 09.03.2013 02:40, schrieb Graham: >> On 08/03/13 22:40, Francois Gouget wrote: >>> On Fri, 8 Mar 2013, Marcus Meissner wrote: >>> [...] >>>> "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? >>>> >>>> - wine uses it own elfloader, which might cause issues. >>> Really? I thought it used the standard elf loader hence the need for >>> tricks to get the native libraries to load in the right place. What it >>> does have however is its own PE loader. Not sure if it's relevant here. >>> >> the functions i detoured in kernel32.dll.so were LoadLibrary and >> FreeLibrary, the question still remains how its even possible, this >> should be a fairly straight-forward thing to answer for you developers, >> i would had thought. If its too much to understand , please say :/ > Wines loader stuff can be fairly complex as Marcus already pointed out, though the change(s) needed to fix your problem was most likely quite small and nothing we keep in mind. > Why didn't you tried using Regression testing as Alexandre Goujon suggested? > >> i put code which detours LoadLibrary and FreeLibrary in kernel32.dll.so >> , but the code only runs at a later certain time after program execution >> ... ( trigger is actually a certain library loaded, i detect it in dlsym >> ) ... But in wine 1.4.1 the program crashes at a time earlier than the >> triggered detour, understand?? This don't seem physically possible, any >> explanation? > Maybe another thread or something was calling these functions before you do, or you called a function that calls one of them and you didn't expected that? > > i have dlsym in my LD_PRELOAD, even if another thread is calling dlsym, my logging within the implementaiton of dlsym in my LD_PRELOAD library would notify me of such? My code is very simple, i check for certain library symbol within dlsym, if its one which is called last ( i remember its name )/ at the end of the loading routine, then i will from there initiate my detour to loadlibrarya.. within loadlibrary a, i create a new thread with pthread_create,, only on condition of a particular windows dll given as argument to loadlibrarya ofc. this is how its working, and it works under 1.5 , not 1.4.1 , i go with the theory its something with the elf loader, cheers ...

March 9, 2013

Re: Crash confuses me

by Graham

On 09/03/13 06:03, Marcus Meissner wrote: > On Sat, Mar 09, 2013 at 01:40:00AM +0000, Graham wrote: >> On 08/03/13 22:40, Francois Gouget wrote: >>> On Fri, 8 Mar 2013, Marcus Meissner wrote: >>> [...] >>>> "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? >>>> >>>> - wine uses it own elfloader, which might cause issues. >>> Really? I thought it used the standard elf loader hence the need for >>> tricks to get the native libraries to load in the right place. What it >>> does have however is its own PE loader. Not sure if it's relevant here. >>> >> the functions i detoured in kernel32.dll.so were LoadLibrary and >> FreeLibrary, the question still remains how its even possible, this >> should be a fairly straight-forward thing to answer for you developers, >> i would had thought. If its too much to understand , please say :/ >> >> i put code which detours LoadLibrary and FreeLibrary in kernel32.dll.so >> , but the code only runs at a later certain time after program execution >> ... ( trigger is actually a certain library loaded, i detect it in dlsym >> ) ... But in wine 1.4.1 the program crashes at a time earlier than the >> triggered detour, understand?? This don't seem physically possible, any >> explanation? > > In loader/preloader.c is our own pre-elf-loader, which handles the specific address > space layouts we use. > > All the library loading is very magic even to me and I have to understand it anew > every time ;) > > > Why not just patch the Wine source itself and build a specific Wine version for > your problem? This makes it easier and avoids need of preloading tricks. > > You probably also want to patch LdrLoadDll in ntdll. > > Ciao, Marcus Gracias. That helps, cheers all. And Good Luck with the continuation of this awesome software

March 9, 2013

Re: Crash confuses me

by André Hentschel

Am 09.03.2013 02:40, schrieb Graham: > On 08/03/13 22:40, Francois Gouget wrote: >> On Fri, 8 Mar 2013, Marcus Meissner wrote: >> [...] >>> "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? >>> >>> - wine uses it own elfloader, which might cause issues. >> Really? I thought it used the standard elf loader hence the need for >> tricks to get the native libraries to load in the right place. What it >> does have however is its own PE loader. Not sure if it's relevant here. >> > > the functions i detoured in kernel32.dll.so were LoadLibrary and > FreeLibrary, the question still remains how its even possible, this > should be a fairly straight-forward thing to answer for you developers, > i would had thought. If its too much to understand , please say :/ Wines loader stuff can be fairly complex as Marcus already pointed out, though the change(s) needed to fix your problem was most likely quite small and nothing we keep in mind. Why didn't you tried using Regression testing as Alexandre Goujon suggested? > i put code which detours LoadLibrary and FreeLibrary in kernel32.dll.so > , but the code only runs at a later certain time after program execution > ... ( trigger is actually a certain library loaded, i detect it in dlsym > ) ... But in wine 1.4.1 the program crashes at a time earlier than the > triggered detour, understand?? This don't seem physically possible, any > explanation? Maybe another thread or something was calling these functions before you do, or you called a function that calls one of them and you didn't expected that? -- Best Regards, André Hentschel

March 9, 2013

Re: comctl32/listview: fix overwriting of item values

by Nikolay Sivov

On 3/9/2013 10:03, Daniel Jelinski wrote: Makes sense, thanks. I briefly searched for such cases and LISTVIEW_FindItemW() seems to be broken the same way.

March 9, 2013

Re: Crash confuses me

by Marcus Meissner

On Sat, Mar 09, 2013 at 01:40:00AM +0000, Graham wrote: > On 08/03/13 22:40, Francois Gouget wrote: > > On Fri, 8 Mar 2013, Marcus Meissner wrote: > > [...] > >> "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? > >> > >> - wine uses it own elfloader, which might cause issues. > > Really? I thought it used the standard elf loader hence the need for > > tricks to get the native libraries to load in the right place. What it > > does have however is its own PE loader. Not sure if it's relevant here. > > > > the functions i detoured in kernel32.dll.so were LoadLibrary and > FreeLibrary, the question still remains how its even possible, this > should be a fairly straight-forward thing to answer for you developers, > i would had thought. If its too much to understand , please say :/ > > i put code which detours LoadLibrary and FreeLibrary in kernel32.dll.so > , but the code only runs at a later certain time after program execution > ... ( trigger is actually a certain library loaded, i detect it in dlsym > ) ... But in wine 1.4.1 the program crashes at a time earlier than the > triggered detour, understand?? This don't seem physically possible, any > explanation? In loader/preloader.c is our own pre-elf-loader, which handles the specific address space layouts we use. All the library loading is very magic even to me and I have to understand it anew every time ;) Why not just patch the Wine source itself and build a specific Wine version for your problem? This makes it easier and avoids need of preloading tricks. You probably also want to patch LdrLoadDll in ntdll. Ciao, Marcus

March 9, 2013

Re: Crash confuses me

by Graham

On 08/03/13 22:40, Francois Gouget wrote: > On Fri, 8 Mar 2013, Marcus Meissner wrote: > [...] >> "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? >> >> - wine uses it own elfloader, which might cause issues. > Really? I thought it used the standard elf loader hence the need for > tricks to get the native libraries to load in the right place. What it > does have however is its own PE loader. Not sure if it's relevant here. > the functions i detoured in kernel32.dll.so were LoadLibrary and FreeLibrary, the question still remains how its even possible, this should be a fairly straight-forward thing to answer for you developers, i would had thought. If its too much to understand , please say :/ i put code which detours LoadLibrary and FreeLibrary in kernel32.dll.so , but the code only runs at a later certain time after program execution ... ( trigger is actually a certain library loaded, i detect it in dlsym ) ... But in wine 1.4.1 the program crashes at a time earlier than the triggered detour, understand?? This don't seem physically possible, any explanation?

March 9, 2013

Re: spec file and -arch

by André Hentschel

Am 05.03.2013 22:57, schrieb Stefan Leichter: > Hi, > > when i look into msvcr100.spec file i find lots of different parameters for the > switch (?) -arch . So fare i have seen: arm, i386, win32, win64 and x86_64 . > > To me it is not clear what is the difference between win64 and x86_64. The same > for i386 and win32. What are is the criteria to choose the first or the second? > If it makes a difference do i have to distinguish between arm and ... ? win32 includes all 32-bit platforms win64 includes all 64-bit platforms if is implemented in x86 assembler win32 would not be the best choice. i suggest you should use make_specfiles in tools/ > Does -i386 and -arch=i386 mean the same? yes -- Best Regards, André Hentschel

March 8, 2013

Re: Crash confuses me

by Francois Gouget

On Fri, 8 Mar 2013, Marcus Meissner wrote: [...] > "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? > > - wine uses it own elfloader, which might cause issues. Really? I thought it used the standard elf loader hence the need for tricks to get the native libraries to load in the right place. What it does have however is its own PE loader. Not sure if it's relevant here. -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ Hiroshima '45 - Czernobyl '86 - Windows '95

March 8, 2013

Re: ntdll: make NtDelayExecution a bit more efficient

by Francois Gouget

On Fri, 8 Mar 2013, Juan Lang wrote: [...] > > Michael Stefaniuc wrote: > > >> i.e. commit 8099c2b9. JW says "... to more closely resemble Windows > > >> behavior. The key is to yield in a Sleep..." I think it's really a three patch series and the first one has the clearer commit message: commit 08c0f691cebc0994bb94bc160c080cccf8068561 Author: Jeremy White <jwhite(a)codeweavers.com> Date: Sat Oct 9 02:26:29 2004 +0000 Made NtDelayExecution with a 0 timeout yield the CPU, as it is supposed to. commit de91a8dd0fd01da20656a4f6be423e5ae2df785f Author: Alexandre Julliard <julliard(a)winehq.org> Date: Mon Oct 11 20:11:01 2004 +0000 Implemented NtYieldExecution. commit 8099c2b9fd5b80f35868936a6e96f732106c3286 Author: Jeremy White <jwhite(a)codeweavers.com> Date: Tue Nov 2 19:32:03 2004 +0000 Tune the behavior of Sleep() and Waitxxx() to more closely resemble Windows behavior. The key is to yield in a Sleep and in any Wait that times out. > > If the ancient wisdom isn't backed by tests there's a fair chance that > > > it might not be applicable today. Or that it was a wrong theory [...] > I no longer recall which app it was Jeremy was working on fixing, but there > was at least one app that depended on this behavior. See Jeremy's posts on > "RFC: yield on Waits" and "Threading issues". http://www.winehq.org/pipermail/wine-devel/2004-November/030796.html I suspect it was the Shockwave Director plugin(*). If I remember correctly it had a thread in a tight loop just calling PeekMessage() and Sleep(0) or some such. Then it had another thread that was doing the real work. But because the first thread was not yielding the second's performance suffered (even more so as the processors of the time where mostly single core). (*) Nothing to do with Shockwave Flash, aka Macromedia Flash aka Adobe Flash. -- Francois Gouget <fgouget(a)codeweavers.com>

March 8, 2013

Re: Crash confuses me

by Marcus Meissner

On Thu, Mar 07, 2013 at 07:53:57PM +0000, Graham wrote: > I use a LD_PRELOAD before running wine so that i can debug my application. > > The library which is preloaded is able to replace system dlsym and from > that, other consequence library call. > > There is no problem until, i put some Detour code(hot patching) into the > kernel32.dll.so address space. But the very funny things is this > ** THE PROGRAM CRASHES BEFORE MY HOTPATCH IS APPLIED ** > > so it seems somehow the order of cause and effect is extremely weird > and frightening to me. How can a piece of code which is not excecuted be > teh cause for errors PRIOR/BEFORE to its execution. And this is surely > happening because when i uncomment the Hotpatch/detour lines the problem > goes away. > > here are some lines from the crashs ... > > err:module:attach_process_dlls "user32.dll" failed to initialize, aborting > err:module:LdrInitializeThunk Main exe initialization for > L"C:\\windows\\system32\\winemenubuilder.exe" failed, status c0000005 > wine: Unhandled page fault on execute access to 0x7c9e4cd7 at address > 0x7c9e4cd7 (thread 000b), starting debugger... > err:module:DelayLoadFailureHook failed to delay load > user32.dll.BroadcastSystemMessageW > wine: Call from 0x7b83ba92 to unimplemented function > user32.dll.BroadcastSystemMessageW, aborting > wine: Unimplemented function user32.dll.BroadcastSystemMessageW called > at address 0x7b83ba92 (thread 001d), starting debugger... > > Another thing to notice is that this was tested using wine1.4.1 > > However it DOES work with wine1.5.10 ... to get something to work for me > is not the challenge, its to understand the logic behind why something > does not. > > THanks a lot and I am grateful for any who contributes his knowledge to > this problem i am a real NOVICE "real NOVICE" and using LD_PRELOAD or "hotpatching" does not fit together for me...? - wine uses it own elfloader, which might cause issues. - during loading/linking it might also interact already. what should the LD_PRELOAD thing achieve? Can perhaps a wine internal debug mechanism be used already? Ciao, Marcus

March 8, 2013

Re: user32/tests: SetWindowPos() propagates update region from WS_CLIPCHILDREN child to its children (try 3).

by Alexandre Julliard

Sergey Guralnik <serhio(a)etersoft.ru> writes: > This patch demonstrates the most interest case from previous versions. > When SetWindowPos() moves child window, that has some invalid > area, it also invalidates children of this window according to its > update region, even if moved window has WS_CLIPCHILDREN style. > > Wine doesn't perform this invalidation. > One pretty large application depends on this feature, and now, > runned under Wine, it has broken output. > > Hope this variant is more clear than all previous. It's better, but testing the message sequence doesn't seem very useful, particularly since you don't differentiate parent and child messages. Also you should test the region before the move to show that it's the move that's changing it. -- Alexandre Julliard julliard(a)winehq.org

March 8, 2013

Re: msvcr110: Add VS2012 C/C++ runtime

by Alexandre Julliard

Stefan Leichter <Stefan.Leichter(a)camline.com> writes: > @@ -0,0 +1,2260 @@ > +# MS VC++2012 runtime library > + > +@ stub -arch=arm ??0?$_SpinWait@$00(a)details@Concurrency@@QAA(a)P6AXXZ@Z > +@ stub -arch=win32 ??0?$_SpinWait@$00(a)details@Concurrency@@QAE(a)P6AXXZ@Z > +@ stub -arch=win64 ??0?$_SpinWait@$00(a)details@Concurrency@@QEAA(a)P6AXXZ@Z > +@ stub -arch=arm ??0?$_SpinWait@$0A@@details(a)Concurrency@@QAA(a)P6AXXZ@Z > +@ stub -arch=win32 ??0?$_SpinWait@$0A@@details(a)Concurrency@@QAE(a)P6AXXZ@Z > +@ stub -arch=win64 ??0?$_SpinWait@$0A@@details(a)Concurrency@@QEAA(a)P6AXXZ@Z > +@ stub -arch=arm,win32 ??0SchedulerPolicy(a)Concurrency@@QAA(a)IZZ That doesn't make much sense, arm is win32 too. -- Alexandre Julliard julliard(a)winehq.org

March 8, 2013

Re: ntdll: make NtDelayExecution a bit more efficient

by Juan Lang

On Fri, Mar 8, 2013 at 6:11 AM, Graham Knap <graham.knap(a)gmail.com> wrote: > Michael Stefaniuc wrote: > >> i.e. commit 8099c2b9. JW says "... to more closely resemble Windows > >> behavior. The key is to yield in a Sleep..." > > > > JW is Jeremy White so us old timers chuckle now ;) > > I know the name, but nothing more. Why is this funny? > He's the CEO of Codeweavers, and he's the first one to admit he doesn't write great code, or at least that Alexandre rejects his code a lot :) > If the ancient wisdom isn't backed by tests there's a fair chance that > > it might not be applicable today. Or that it was a wrong theory > > Interesting. Unfortunately, I have no idea how to go about writing > tests for this. Can anyone offer any suggestions? > It's, well, hard to test. MSDN suggests it was true in XP, but subsequently changed: http://msdn.microsoft.com/en-us/library/windows/desktop/ms686298(v=vs.85).a… I no longer recall which app it was Jeremy was working on fixing, but there was at least one app that depended on this behavior. See Jeremy's posts on "RFC: yield on Waits" and "Threading issues". --Juan

March 8, 2013

Re: winhttp: Implement WINHTTP_OPTION_CONNECTION_INFO. (try 2)

by Alexandre Julliard

Hans Leidekker <hans(a)codeweavers.com> writes: > --- > dlls/winhttp/session.c | 71 ++++++++++++++++++++++++++++++++++++++++++ > dlls/winhttp/tests/winhttp.c | 62 ++++++++++++++++++++++++++++++++++++ > include/winhttp.h | 16 ++++++++++ > 3 files changed, 149 insertions(+) Now it breaks the Android build... i686-linux-android-gcc -c -I../../../../wine/dlls/winhttp -I. -I../../../../wine/include -I../../include -D__WINESRC__ -D_REENTRANT -fPIC -Wall -pipe -fno-strength-reduce -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wwrite-strings -gdwarf-2 -gstrict-dwarf -fno-omit-frame-pointer -Wpointer-arith -Wlogical-op -I/usr/include/freetype2 -g -O2 -o session.o ../../../../wine/dlls/winhttp/session.c ../../../../wine/dlls/winhttp/session.c: In function 'convert_sockaddr': ../../../../wine/dlls/winhttp/session.c:609:25: error: 'struct WS_sockaddr_in' has no member named '__pad' make[1]: *** [session.o] Error 1 -- Alexandre Julliard julliard(a)winehq.org

March 8, 2013

Re: ntdll: make NtDelayExecution a bit more efficient

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Am 2013-03-08 15:11, schrieb Graham Knap: > Michael Stefaniuc wrote: >> If the ancient wisdom isn't backed by tests there's a fair chance >> that it might not be applicable today. Or that it was a wrong >> theory This is guesswork, but maybe the yield is there to make code like this work (or more likely to work): start = GetTickCount(); Sleep(0); end = GetTickCount(); output = input / (end - start); -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJROfmEAAoJEN0/YqbEcdMwetYP/3NB0CEhVB0VtTa4jfxGOdx8 cMFguVUpxjVAHoV6FlovydW4Eb/ERGsjn8HFocabuyvLkMshzwt22AZC4REfaibZ zAywbrdjbu0FQEURlhMuDw+jZvEoqNRmYBSWq5omxB+HuWu6YD+Hkw9zfl7AKdSa lrCFilJ3dFscY5NTViZN75fAXh7HrYKUjXRiPUln5qUBFvKcSQniUXiaLheIgSRA UqllLxuTAX0wKQkYrxGBiPcabkR3Rdn6JyIzWytmbUS3PCJYrWbPvRdV+/Rn4RjY YGvrMzffJGQu6MlwZkxi8gr/tZc6p0AWkJ4EmHd6zYipJJrKYER9GVCsmrex15BR 4d6ebL9tB3jVboAb9rLNl1ZPHKUCB8no9+uxFYJv5+cFAspJstFzupW18z2LB5X+ DCzemYXlW3dWueFxNM+1PjE3vSAL+5lRkmhKEL+7OMPXC5jRP2XDWauVYHrSX1oF ZmgfjU7/VNl4Li1grqLP8Ux0vv1RLPsNfar8ycByBh4dCpzv+ZL3MnkueSbryqBH TEvSr4WIDe3YVDjxwNdbiSGUFEwgILfrZC4SLkR8NOVu2Nl26z1wsbE4vk2XTMGs 9EBMXsp5EdFnvK3etNphrWcJYJKA6P0GlFF/iXFzR2z3UKQBddQLw/LnFmaEprx2 20fKvxaGI1pIlGBi3Hl+ =p/ep -----END PGP SIGNATURE-----

March 8, 2013

Re: ntdll: make NtDelayExecution a bit more efficient

by Graham Knap

Michael Stefaniuc wrote: >> i.e. commit 8099c2b9. JW says "... to more closely resemble Windows >> behavior. The key is to yield in a Sleep..." > > JW is Jeremy White so us old timers chuckle now ;) I know the name, but nothing more. Why is this funny? > If the ancient wisdom isn't backed by tests there's a fair chance that > it might not be applicable today. Or that it was a wrong theory Interesting. Unfortunately, I have no idea how to go about writing tests for this. Can anyone offer any suggestions? -- graham

March 8, 2013

[1/3] Make mac driver the default on OS X

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, >> I'm not sure it's worth it. In my experience, the virtual desktop >> has more often been used to work around X window manager >> limitations. The hope is that we have greater control with the Mac >> driver. If all else fails, the X11 driver is still available. > True. Anyone who'd even bother to set up a virtual desktop would > probably be willing to go to the trouble of installing XQuartz > anyway. I completely disagree. I start *all* games with a virtual desktop. Here's why. When I sarted on the Mac 3 years ago with Wine, xrandr was not available, so the only full screen resolution was the monitor's full 1600x1200 (or rather 1178). None of the full screen games would work with that, they were expecting 800x600, 1024x768 or 1440x1050. As a result, I started each graphics app with a virtual desktop in a resolution that would make it work well. Initially I considered this solution to be inferior to the full screen we all know from X11 on Linux, however I very soon got used to it. Actually, it's very nice to run all games in a window and it works pleasantly well. E.g. Motocross Madness' maximum 1440x1050 window leaves enough room within 1600x1200 for the activity monitor and a few other displays. Wine manages well to keep the mouse within the window when needed, and there's always Cmd-Tab to switch to another app. A shared screen is also easier than having to remember the ctrl-alt-a or whatever key that can toggle between the Apple and X11 screens. At some time, XQuartz added xrandr support. I was not satisfied with it, as I soon found out that while some games may work with the full 1600x1200, they would not display their intro videos that expected a 800x600 screen or worse, would crash at the intro. Also, I did not want apps to switch my monitor's resolution to meager 800x600. The resizable virtual desktop still gives best results: it opens a 800x600 window for the intro videos, then resizes during game. Motocross Madness' menu screen is a fixed 640x480 IIRC and it's only during race that it will resize to 1440x1050. >From today's perspective, I don't always want a full screen 640x480. Actually, it depends on the app. Some apps benefit from the monitor's scaling to full dimensions. Some zoomed content looks awful on today's huge monitors. Some 1024x768 apps look like postal stamps inside a large screen whereas other 1024x768 window contents looks good. The virtual desktop puts the choice in the user's hands. Sometimes I've used Leopard's zooming feature (cmd-shift-8 IIRC). Also, with apps as windows, the children can iconify them anytime and start a browser, or go and eat, then resume later, trivially. Regards, Jörg Höhle

March 8, 2013

[1/3] Make mac driver the default on OS X

by Joerg-Cyril.Hoehle＠t-systems.com

Hi, Charles Davis wrote: >While we're on the subject of bugs: another annoying problem is that, >when a window is first created, the view appears black I had a very annoying experience: when I started winemac the first time, the whole screen turned black for a fraction of a second. That was very unwelcome and reminded me of the infamous Intel-XOrg screen flickering issue that plagued many Linux distros a few years ago :-( >I'll bet Jörg is running Snow Leopard Indeed, with newest updates (10.6.8). Note that I'm running Apple X11, not XQuartz. While that X11 server version is older that the 2.6.3 that I used on Leopard 10.5.8, it's nevertheless newer than the first XQuartz that I used with Wine after I got my "early 2009" nVidia Mac Mini 3 years ago. For the moment, I've decided to stick to Apple X11 and see how far it goes. >There is a white grow box for me in the lower-right corner when I use >the X11 driver. He must have been using a different WM than the >default quartz-wm. See above. I used notepad etc. with wine-1.5.25. > Another annoyance was that there's no HIDE function or short cut and > some dialogues do not provide the orange button (middle one) to > iconify the window. You all embarked on the Cmd/Alt-H issue. A menu, even without short- cut, would be nice to have. Regarding the Alt key, XQuartz has some keyboard configuration option in its settings menu. Maybe winemac should provide a similar choice, such that X11 and winemac provide identical behavior and ease switching? Regards, Jörg Höhle

March 8, 2013

Re: Crash confuses me

by GOUJON Alexandre

On 03/07/2013 08:53 PM, Graham wrote: > <snip> > Another thing to notice is that this was tested using wine1.4.1 > > However it DOES work with wine1.5.10 [...] > <snip> If it works with the latest version, it means there was a bug and we fixed it. Each commit is as small as possible to ease the identification of the issue when we need to do a regresssion testing. So, if you want to know what commit fixed the issue and what was the issue, follow the guide : http://wiki.winehq.org/RegressionTesting Hope that helps

March 8, 2013