Re: urlmon: Fix incorrect pointer arithmetic (too conversative) in map_url_to_zone. (RESEND)
On Jun 4, 2011, at 8:02 AM, Gerald Pfeifer wrote:
> Resending: This really looks like a straightforward bug fix and the
> current code definitely wrong???
No. As others have pointed out, your logic is wrong. The existing code is
correct.
> The difference between two pointers (of the same type) is the number
> of elements, not the number of bytes. Thus the code below was way
> incorrect, luckily only too conversative.
So, ptr-path is the number of elements between the two pointers. But
sizeof(root) is a number of bytes. The precise reason to divide the latter by
sizeof(WCHAR) is to arrive at a number of elements so it is proper to compare
to ptr-path.
Put another way, look a bit lower in the code:
> memcpy(root, path, (ptr-path)*sizeof(WCHAR));
It is clear that (ptr-path)*sizeof(WCHAR), a measure of bytes, must be no
larger than the size of root in bytes. Thus, this is the requirement:
(ptr-path)*sizeof(WCHAR) <= sizeof(root)
Dividing both sides by sizeof(WCHAR) gives an equivalent requirement:
(ptr-path) <= sizeof(root)/sizeof(WCHAR)
which is exactly what the code, as is, tests. (Except that the current code
doesn't allow for the equal case, in order to preserve a null terminator.)
Regards,
Ken
> ---
> dlls/urlmon/sec_mgr.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/urlmon/sec_mgr.c b/dlls/urlmon/sec_mgr.c
> index 7b4bb35..75850ee 100644
> --- a/dlls/urlmon/sec_mgr.c
> +++ b/dlls/urlmon/sec_mgr.c
> @@ -529,7 +529,7 @@ static HRESULT map_url_to_zone(LPCWSTR url, DWORD *zone,
> LPWSTR *ret_url)
> hres = CoInternetParseUrl(secur_url, PARSE_PATH_FROM_URL, 0, path,
> sizeof(path)/sizeof(WCHAR), &size, 0);
>
> -if(SUCCEEDED(hres) && (ptr = strchrW(path, '\\')) && ptr-path <
> sizeof(root)/sizeof(WCHAR)) {
> +if(SUCCEEDED(hres) && (ptr = strchrW(path, '\\')) && ptr-path <
> sizeof(root)) {
> UINT type;
>
> memcpy(root, path, (ptr-path)*sizeof(WCHAR));
> --
> 1.7.4.1
>
>
>
Re: wineoss.drv: Add mmdevapi driver.
On Mon, 9 May 2011, Andrew Eikum wrote: > Makes sense to me. Thanks. Do this patch and the float format patch > together fix your build issues, Gerald? The short answer is: Yes! The long answer is that here have been three different issues around OSS on current versions of FreeBSD where out of the box Wine would not build. 1. Lack of AFMT_FLOAT This has been addressed by Wine commit bed73e9e736ef2376e762730a30ad5f2611f969c of mine which only uses this if present. I also filed the following report against FreeBSD: http://www.freebsd.org/cgi/query-pr.cgi?pr=157050 2. Lack of AFMT_S24_PACKED This has been addressed by Wine commit 051b64b66f82801d43016068639468575dc3974e of mine which replaced it with AFMT_S24_LE. I have not filled a bug against FreeBSD since according to http://manuals.opensound.com/developer/AFMT_S24_PACKED.html this does not appear all too useful to begin with? 3. Lack of SNDCTL_DSP_HALT. This is addressed by my latest patch that uses the equivalent, but older and deprecated, SNDCTL_DSP_RESET in case. I also submitted a patch against FreeBSD along the same lines: http://www.freebsd.org/cgi/query-pr.cgi?pr=156874 As you can see, all three issues have been addressed in Wine now and for two of them I filed bugs against FreeBSD (one with a patch). :-) Thanks for your guidance and support around this! Gerald
Re: wineoss.drv: Support platforms that do not feature AFMT_FLOAT.
On Mon, 9 May 2011, Andrew Eikum wrote: > Anyways this is a tiny hassle, so I'm fine with it. Perhaps you should > consider filing a FreeBSD bug? Sure thing, Andrew! That's sounds like a good idea (no pun intended :-). http://www.freebsd.org/cgi/query-pr.cgi?pr=157050 Gerald
Re: Added unbound sampler test for pixel shaders. (try 2)
Testing 3D and Cube textures (provided they're supported) may be useful too.
Re: wined3d: Enable blit stretching for 64 bpp
On Saturday 04 June 2011 18:24:39 André Hentschel wrote: > +case 8: > +STRETCH_ROW(DWORD64); > +break; 64 bit formats can be floating point formats too, like ARGB16F, or RG32F. But Roderick's comment catches the more important issue: This code shouldn't be called for 64 bit formats at all since they are only supported with FBOs which bring hardware blitting support. signature.asc Description: This is a digitally signed message part.
re: gdi speed: 10x slower than real win
Yup. It's being worked on, slowly; see http://wiki.winehq.org/Performance, especially http://wiki.winehq.org/DIBEngine
Re: [1/8] d3d8: Use D3DPOOL <-> WINED3DPOOL enum conversion functions instead of implicit conversions (clang)
On Sat, Jun 4, 2011 at 23:50, Henri Verbeet wrote: > Conceptually I think it makes sense to have a distinction between > wined3d types and e.g. ddraw types. For things like D3DPOOL that's not > strictly required since it has the same values in all d3d versions > where it exists, but e.g. format IDs are incompatible between > versions. In that sense calling it WINED3DPOOL instead of D3DPOOL is > mostly just a matter of consistency. On the other hand, perhaps the > more fundamental point is that I'm not so sure that "superset of all > d3d versions" is the correct level of abstraction for wined3d. I > suspect we'll see that break down a bit as we implement more of > d3d10/11, and perhaps also with some ddraw cleanups. (To pick a random > example, consider d3d10/11 sampler states vs d3d8/9 sampler states.) > > Note that WINED3DPOOL is a terrible example though, since it should go > away in favor of d3d10 style CPU/GPU access masks in the medium to > long term anyway. Similarly, it makes much more sense to fix something > like IDirect3DTexture8Impl_GetType() by replacing most of the function > with just "return D3DRTYPE_TEXTURE;" instead of introducing the > d3dresourcetype_from_wined3dresourcetype() function. The existing code > should be perfectly valid C, though perhaps not all that pretty in > some places. Clang throws some warnings, but well, tough. It's not > very high on my list of things I want to spend time thinking about. At > this point we probably spent more time talking about it than fixing > those enum conversion warnings would ever save anyone. Except for the > cases that are obvious to fix / improve, I think it's probably best to > just leave it alone. OK. In that case I'll just abandon this patch series altogether, since it's probably a waste of effort anyway to try and get something in. However, with or without warnings/clang, passing around non 1-1 mapped enum values is still bad style IMHO, and erroneous values can still be passed unnoticed. >Maybe it will go away if we ignore it. Definitely when some cleanup is eventually done, but that's an orthogonal argument. Frédéric Delanoy
Re: dinput: Added a check for NULL callback in EnumDevices.
On 06/04/2011 01:32 PM, Lucas Zawacki wrote: Sometimes apps depend on the crash. Since your change comes with a test case, it looks reasonable to me, although you might check the specific HRESULT rather than just FAILED. Exactly, it's not a crash on Windows. I'll resend it with an explicit test. I was thinking of this more as "janitorial" work as opposed to fixing a buggy app, maybe my comment worded it badly. AJ was explicit about such checks: unless you have an application that breaks because of this don't add any null checks. They needlessly slowdown Wine. And won't guard you against invalid pointer anyway. Vitaliy.
Re: cmd: ensure create_full_path returns FALSE for all errors
2011/6/4 Frédéric Delanoy : > BTW, there's a > +rem warning, don't run any tests that depend on %ErrorLevel% after this line > in your patch > > Is there really no way to reset ErrorLevel to a sane value? It's more subtle than that. That section of code checks what happens when you try to set an environment variable named ErrorLevel, which is kind of a no-no; real world batch files shouldn't be doing that, as it doesn't do what one expects. > IMHO it makes more sense to use tests using ErrorLevel *after* its > correct behaviour has been demonstrated, if possible... Order should not be important, IMHO. We ought to be running each of those sections in its own little cmd session. That would prevent test failures from cascading into the next section. - Dan
Re: cmd: ensure create_full_path returns FALSE for all errors
2011/6/4 Dan Kegel : > 2011/6/4 Dan Kegel : >> https://testbot.winehq.org/JobDetails.pl?Key=11484 >> It passes on all tested platforms. Feel free to >> submit along with your patch verbatim if it looks >> sufficient to you. > > BTW it looks like cmd test suite completely ignores > stderr, which explains why that test passes even though > the error message produced by wine's cmd and native cmd > are different. BTW, there's a +rem warning, don't run any tests that depend on %ErrorLevel% after this line in your patch Is there really no way to reset ErrorLevel to a sane value? IMHO it makes more sense to use tests using ErrorLevel *after* its correct behaviour has been demonstrated, if possible...
Re: [1/8] d3d8: Use D3DPOOL <-> WINED3DPOOL enum conversion functions instead of implicit conversions (clang)
On 31 May 2011 12:50, Alexandre Julliard wrote: > I mean using D3DPOOL in wined3d, not by including the d3d9 headers, but > by defining it when necessary, i.e. if neither d3d8types.h nor > d3d9types.h have been included already. > > This way d3d8 uses the d3d8 definition, d3d9 uses the d3d9 definition, > and ddraw/d3d10/wined3d internals use the wined3d definition which is > probably a superset of the others. > It's called WINED3DPOOL instead of D3DPOOL since it's a wined3d type, but otherwise I think that's close enough to what we currently have that it's practically the same. Conceptually I think it makes sense to have a distinction between wined3d types and e.g. ddraw types. For things like D3DPOOL that's not strictly required since it has the same values in all d3d versions where it exists, but e.g. format IDs are incompatible between versions. In that sense calling it WINED3DPOOL instead of D3DPOOL is mostly just a matter of consistency. On the other hand, perhaps the more fundamental point is that I'm not so sure that "superset of all d3d versions" is the correct level of abstraction for wined3d. I suspect we'll see that break down a bit as we implement more of d3d10/11, and perhaps also with some ddraw cleanups. (To pick a random example, consider d3d10/11 sampler states vs d3d8/9 sampler states.) Note that WINED3DPOOL is a terrible example though, since it should go away in favor of d3d10 style CPU/GPU access masks in the medium to long term anyway. Similarly, it makes much more sense to fix something like IDirect3DTexture8Impl_GetType() by replacing most of the function with just "return D3DRTYPE_TEXTURE;" instead of introducing the d3dresourcetype_from_wined3dresourcetype() function. The existing code should be perfectly valid C, though perhaps not all that pretty in some places. Clang throws some warnings, but well, tough. It's not very high on my list of things I want to spend time thinking about. At this point we probably spent more time talking about it than fixing those enum conversion warnings would ever save anyone. Except for the cases that are obvious to fix / improve, I think it's probably best to just leave it alone. Maybe it will go away if we ignore it.
Re: cmd: ensure create_full_path returns FALSE for all errors
2011/6/4 Dan Kegel : > https://testbot.winehq.org/JobDetails.pl?Key=11484 > It passes on all tested platforms. Feel free to > submit along with your patch verbatim if it looks > sufficient to you. BTW it looks like cmd test suite completely ignores stderr, which explains why that test passes even though the error message produced by wine's cmd and native cmd are different.
Re: cmd: ensure create_full_path returns FALSE for all errors
2011/6/4 Frédéric Delanoy : > On Sat, Jun 4, 2011 at 19:27, Dan Kegel wrote: >> Please add a test case, too. > > OK. Do I need a testbot account as well? Yeah. The cmd test suite is touchy, so it's worth it to use testbot to verify your test before you send it to wine-patches. I threw an example test case together for you. A quick run on xp seems to indicate you also need to set errorlevel, so I added that. See the patch attached to https://testbot.winehq.org/JobDetails.pl?Key=11484 It passes on all tested platforms. Feel free to submit along with your patch verbatim if it looks sufficient to you. (Oddly, windows xp's cmd doesn't seem to set errorlevel if you use rmdir on a nonexistent directory...) - Dan
Re: dinput: Added a check for NULL callback in EnumDevices.
> Sometimes apps depend on the crash. Since your change comes with a > test case, it looks reasonable to me, although you might check the > specific HRESULT rather than just FAILED. Exactly, it's not a crash on Windows. I'll resend it with an explicit test. I was thinking of this more as "janitorial" work as opposed to fixing a buggy app, maybe my comment worded it badly.
Re: cmd: ensure create_full_path returns FALSE for all errors
On Sat, Jun 4, 2011 at 19:27, Dan Kegel wrote: > Please add a test case, too. OK. Do I need a testbot account as well?
Re: dinput: Added a check for NULL callback in EnumDevices.
> I don't. I just thought that a case where Wine misbehaving causes a > crash was worth fixing. Sometimes apps depend on the crash. Since your change comes with a test case, it looks reasonable to me, although you might check the specific HRESULT rather than just FAILED. --Juan
Re: dinput: Added a check for NULL callback in EnumDevices.
> Do you have an example of such application? And the bug in bugzilla? > I don't. I just thought that a case where Wine misbehaving causes a crash was worth fixing.
re: cmd: ensure create_full_path returns FALSE for all errors
Please add a test case, too.
Re: wined3d: Enable blit stretching for 64 bpp
Hi André,
I'm not sure if we should add this change since for a part it hides
the real problem. Sure this patch likely works, but >32bpp blits
should never enter the software rendering code in the first place. The
code used to be a huge mess and Henri cleaned it up quite a bit
already (thanks, because I became too busy). It may be easier now to
fix this properly.
Roderick
2011/6/4 André Hentschel :
> For http://bugs.winehq.org/show_bug.cgi?id=27377
> ---
> dlls/wined3d/surface.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c
> index 91b16a6..7f9d9df 100644
> --- a/dlls/wined3d/surface.c
> +++ b/dlls/wined3d/surface.c
> @@ -6780,6 +6780,9 @@ do { \
> case 4:
> STRETCH_ROW(DWORD);
> break;
> + case 8:
> + STRETCH_ROW(DWORD64);
> + break;
> case 3:
> {
> const BYTE *s;
> --
>
> Best Regards, André Hentschel
>
>
>
Re: cmd: ensure create_full_path returns FALSE for all errors
On Sat, Jun 4, 2011 at 16:51, Marcus Meissner wrote:
> On Sat, Jun 04, 2011 at 04:37:05PM +0200, Frédéric Delanoy wrote:
>> This resolves the issue from bug #27383
>> ---
>> programs/cmd/builtins.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
>> index 0352e59..18e16fe 100644
>> --- a/programs/cmd/builtins.c
>> +++ b/programs/cmd/builtins.c
>> @@ -496,6 +496,7 @@ static BOOL create_full_path(WCHAR* path)
>> WCHAR *slash;
>> DWORD last_error = GetLastError();
>> if (last_error == ERROR_ALREADY_EXISTS)
>> + ret = FALSE;
>> break;
>
> You at least miss { } here.
Damn... /me feels stupid... :(
Resending
Re: cmd: ensure create_full_path returns FALSE for all errors
On Sat, Jun 04, 2011 at 04:37:05PM +0200, Frédéric Delanoy wrote:
> This resolves the issue from bug #27383
> ---
> programs/cmd/builtins.c |1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
> index 0352e59..18e16fe 100644
> --- a/programs/cmd/builtins.c
> +++ b/programs/cmd/builtins.c
> @@ -496,6 +496,7 @@ static BOOL create_full_path(WCHAR* path)
> WCHAR *slash;
> DWORD last_error = GetLastError();
> if (last_error == ERROR_ALREADY_EXISTS)
> +ret = FALSE;
> break;
You at least miss { } here.
Ciao, Marcus
Re: [1/2] d3d8: Increment the reference count of the IDirect3D8 parent when creating a device.
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 The two patches look ok Am 03.06.2011 um 14:23 schrieb Andrew Nguyen: > --- > dlls/d3d8/d3d8_private.h |5 +++-- > dlls/d3d8/device.c | 29 - > dlls/d3d8/directx.c |2 +- > dlls/d3d8/tests/device.c | 12 > 4 files changed, 24 insertions(+), 24 deletions(-) > > > <0001-d3d8-Increment-the-reference-count-of-the-IDirect3D8-p.txt> -BEGIN PGP SIGNATURE- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) iQIcBAEBAgAGBQJN6jEJAAoJEN0/YqbEcdMwpNoP/33vRTq283VUfbwKhtVwYGQP hkp1Hh53id7moMGQ4YoH6BhsdWH2KeAvx5asuHEiciJZ/6JiS5Jc/1tNIuD5U+HS RcaFsOrKInuvbPojZECNoe1J/JH5i/A7fCLrAY+FknXOWz+t1UsD6sQbaZqVRM4H j7SvP335TRYYN/8KOyuI2vc2ZDQ+aLo+MXoQpu/7OfRbnnbCsR2ddDLCw/B6ktDN W0fUHjxRU5OigKebUG/KIcXmxYIbbVvNABR4UpRG30Py68CXlYsSM04HRZK4rLea H9pTGSLN1b2eybDKeDbwuIcCVsZHEDFp/mCYxQhPnGoPMHvwfUcHgP8q/rJrTN8+ JYWWYq4g9CuawMxjKivycq30PmAjl2vFl2Dttt9eDuub49ND6sqdDEmcJm8evvhn edOaV8C1o8TVQ4i4AOuUGyMjP3LdXrE02Ec60qjGq5J94NLolAAj5qjDX1JhwghW NKeSsNKGAdstD1FUBOZ6kOPWJ9OOFWyA0GQjiJ7/6616lbjvxJqso/D7IariJImz k/8Rb5UYgYOfB5sHXcyU0pohjouXBqntC5la0Bd1EMy44lZ61K6b03e7rpsyFTKI Wq3Noq1LE4u0gr0E00jE91UVyo30a3Msr6Ss8p1DJf/wmxbrUWNODvYXVlLUd06+ 18tpvn6fFHzrE9vBq3Ll =tSUa -END PGP SIGNATURE-
Re: loader: Add Polish translation
2011/6/3 Łukasz Wojniłowicz : > --- > loader/wine.pl.man.in | 327 > + > 1 files changed, 327 insertions(+), 0 deletions(-) > create mode 100644 loader/wine.pl.man.in > > diff --git a/loader/wine.pl.man.in b/loader/wine.pl.man.in > new file mode 100644 > index 000..e4a14a8 > --- /dev/null > +++ b/loader/wine.pl.man.in > @@ -0,0 +1,327 @@ > +.\" -*- nroff -*- > +.TH WINE 1 "June 2011" "@PACKAGE_STRING@" "Windows On Unix" That line is not translated Also, you might want to alter the Makefile.in for install/uninstalls Frédéric
