RE: Wine users with a CD-ROM drive, please test

[Ge van Geldorp]

> From: Joerg Hoehle
>
> There's no mention of win95/98 in the attachment so I assume these
> were not included, or was that because there was no error?

Right, they were not included. I ran into some problems trying to attach
the .iso to those VMs and gave up. Win9x should die :-).

Ge.

RFC: Fixing kernel32:locale tests on Vista and higher

[Ge van Geldorp]

There are quite a few failures in kernel32:locale on Vista+. The attached
patch tries to address those. Although any comment on the patch is
welcome, I'm particularly interested in feedback on the changes to
test_FoldStringW() (the part that starts at "@@ -2081,43 +2136,49 @@
static void test_FoldStringW(void)".)

The problem with this test was that FoldStringW() behaves very differently
on Vista+ for the MAP_FOLDCZONE and MAP_EXPAND_LIGATURES cases. The tests
were broken off after more than 50 failures, if I remove that then I get
7455 failures for MAP_FOLDCZONE and 1350 failures for
MAP_EXPAND_LIGATURES.

Now, it is possible to fix all those failures. However, doing so would
require us to duplicate the tables that Microsoft uses. This is basically
the same problem that we have with the collation tables. To work around
this, I simply skip the test on Vista+, just like we skip the sorting
test. Does this sound like the right approach?

Thanks, Ge.

diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c
index 8a56100..7acfb90 100644
--- a/dlls/kernel32/tests/locale.c
+++ b/dlls/kernel32/tests/locale.c
@@ -125,7 +125,9 @@ static void test_GetLocaleInfoA(void)
 
   ok(lcid == 0x409, "wrong LCID calculated - %d\n", lcid);
 
-  /* en, ar and zh use SUBLANG_NEUTRAL, but GetLocaleInfo assume SUBLANG_DEFAULT */
+  /* en and ar use SUBLANG_NEUTRAL, but GetLocaleInfo assume SUBLANG_DEFAULT
+ Same is true for zh on pre-Vista, but on Vista and higher GetLocaleInfo
+ assumes SUBLANG_NEUTRAL for zh */
   memset(expected, 0, COUNTOF(expected));
   len = GetLocaleInfoA(MAKELANGID(LANG_ENGLISH, SUBLANG_DEFAULT), LOCALE_SLANGUAGE, expected, COUNTOF(expected));
   SetLastError(0xdeadbeef);
@@ -148,19 +150,6 @@ static void test_GetLocaleInfoA(void)
   else
   win_skip("LANG_ARABIC not installed\n");
 
-  memset(expected, 0, COUNTOF(expected));
-  len = GetLocaleInfoA(MAKELANGID(LANG_CHINESE, SUBLANG_DEFAULT), LOCALE_SLANGUAGE, expected, COUNTOF(expected));
-  if (len) {
-  SetLastError(0xdeadbeef);
-  memset(buffer, 0, COUNTOF(buffer));
-  ret = GetLocaleInfoA(LANG_CHINESE, LOCALE_SLANGUAGE, buffer, COUNTOF(buffer));
-  ok((ret == len) && !lstrcmpA(buffer, expected),
-  "got %d with '%s' (expected %d with '%s')\n",
-  ret, buffer, len, expected);
-  }
-  else
-  win_skip("LANG_CHINESE not installed\n");
-
   /* SUBLANG_DEFAULT is required for mlang.dll, but optional for GetLocaleInfo */
   memset(expected, 0, COUNTOF(expected));
   len = GetLocaleInfoA(MAKELANGID(LANG_GERMAN, SUBLANG_DEFAULT), LOCALE_SLANGUAGE, expected, COUNTOF(expected));
@@ -1300,15 +1289,8 @@ static void test_LCMapStringA(void)
 ok(ret == ret2, "lengths of sort keys must be equal\n");
 ok(!lstrcmpA(buf, buf2), "sort keys must be equal\n");
 
-/* test LCMAP_SORTKEY | NORM_IGNORENONSPACE */
-ret = LCMapStringA(LOCALE_USER_DEFAULT, LCMAP_SORTKEY | NORM_IGNORENONSPACE,
-   lower_case, -1, buf, sizeof(buf));
-ok(ret, "LCMapStringA must succeed\n");
-ret2 = LCMapStringA(LOCALE_USER_DEFAULT, LCMAP_SORTKEY,
-   lower_case, -1, buf2, sizeof(buf2));
-ok(ret2, "LCMapStringA must succeed\n");
-ok(ret == ret2, "lengths of sort keys must be equal\n");
-ok(!lstrcmpA(buf, buf2), "sort keys must be equal\n");
+/* Don't test LCMAP_SORTKEY | NORM_IGNORENONSPACE, produces different
+   results from plain LCMAP_SORTKEY on Vista */
 
 /* test LCMAP_SORTKEY | NORM_IGNORESYMBOLS */
 ret = LCMapStringA(LOCALE_USER_DEFAULT, LCMAP_SORTKEY | NORM_IGNORESYMBOLS,
@@ -1361,9 +1343,14 @@ static void test_LCMapStringW(void)
 win_skip("LCMapStringW is not implemented\n");
 return;
 }
-ok(!ret, "LCMAP_LOWERCASE and LCMAP_UPPERCASE are mutually exclusive\n");
-ok(GetLastError() == ERROR_INVALID_FLAGS,
-   "unexpected error code %d\n", GetLastError());
+if (broken(ret))
+ok(lstrcmpW(buf, upper_case) == 0, "Expected upper case string\n");
+else
+{
+ok(!ret, "LCMAP_LOWERCASE and LCMAP_UPPERCASE are mutually exclusive\n");
+ok(GetLastError() == ERROR_INVALID_FLAGS,
+   "unexpected error code %d\n", GetLastError());
+}
 
 ret = LCMapStringW(LOCALE_USER_DEFAULT, LCMAP_HIRAGANA | LCMAP_KATAKANA,
upper_case, -1, buf, sizeof(buf)/sizeof(WCHAR));
@@ -1459,15 +1446,8 @@ static void test_LCMapStringW(void)
 ok(ret == ret2, "lengths of sort keys must be equal\n");
 ok(!lstrcmpA(p_buf, p_buf2), "sort keys must be equal\n");
 
-/* test LCMAP_SORTKEY | NORM_IGNORENONSPACE */
-ret = LCMapStringW(LOCALE_USER_DEFAULT, LCMAP_SORTKEY | NORM_IGNORENONSPACE,
-   lower_case, -1, buf, sizeof(buf));
-ok(ret, "LCMapStringW must succeed\n");
-ret2 = LCMapStringW(LOCALE_USER_DEFAULT, LCMAP_SORTKEY,
-   lower_case, -1, buf2, sizeof(buf2));
-ok(ret2, "LCMapStringW must succeed\n");
-ok(ret

RE: Crash in the mlang test

[Ge van Geldorp]

Hi Detlef,

> The mlang tests where reordered and now it crashed on
> on your Win98 machine with mlang.dll "4.72.3110.0".
> 
>
http://test.winehq.org/data/675e6e93b1d2b4555d05e311764510abf763d21d/98_gvg-
w98/mlang:mlang.html
>
> Please send me the results for the test with:
> set WINETEST_REPORT_SUCCESS=1

Attached.

Ge.
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1143: Test succeeded
mlang.c:1147: Test succeeded
mlang.c:1154: Test succeeded
mlang.c:1157: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1238: Test succeeded
mlang.c:1226: Test succeeded
mlang.c:1247: Test succeeded
mlang.c:1250: Test succeeded
mlang.c:1255: Test succeeded
mlang.c:1260: Test succeeded
mlang.c:1512: Test succeeded
mlang.c:1517: Test failed: Expected 0, got 4
mlang.c:1519: Test succeeded
mlang.c:1521: Test succeeded

RE: [comctl32] Test request

[Ge van Geldorp]

Hi Nikolay,

>From: Nikolay Sivov [mailto:bungleh...@gmail.com] 
>
>Ge van Geldorp wrote:
>>> From: Nikolay Sivov 
>>>
>>> I've just prepared some simple MRU tests and I need test them on various
>>> native platfoms -
>>>   
>>> tested only on xp sp2 so this one could be skipped.
>>>
>>> Could somebody do it?
>>
>> Your tests pass on my W2K, XP, W2K3, Vista and W2K8 machines, but I get
>> crashes which were not there before on NT4 Workstation and NT4 Server
(both
>> SP6). I haven't investigated further, I could probably determine the test
>> case that crashes if needed.
>
>Thanks for testing, Ge.
>
>Yes, it will be nice if you tell me which tests crash. I'll comment them 
>out then with some notes.

Both the pEnumMRUList(NULL, 0, NULL, 0) and pEnumMRUListW(NULL, 0, NULL, 0)
calls in test_EnumMRUList() cause a crash. If I comment out these 2 tests
then the rest of the tests pass.

Ge.

RE: [comctl32] Test request

[Ge van Geldorp]

> From: Nikolay Sivov 
>
> I've just prepared some simple MRU tests and I need test them on various
native platfoms -
> tested only on xp sp2 so this one could be skipped.
>
> Could somebody do it?

Your tests pass on my W2K, XP, W2K3, Vista and W2K8 machines, but I get
crashes which were not there before on NT4 Workstation and NT4 Server (both
SP6). I haven't investigated further, I could probably determine the test
case that crashes if needed.

Ge.

RE: Getting my test for DrawIcon and DrawIconEx included

[Ge van Geldorp]

Hi Joel,

> From: Joel Holdsworth
>
> Is there a reason why my patch (attached) hasn't been included yet? This
> is my first patch, so maybe someone can help me get it in.

You are aware that this patch introduces test failures on Windows 2000,
Vista and 2008? See
http://www.winehq.org/pipermail/wine-devel/2009-May/075324.html

Ge.

RE: Question about my user32 tests patch

[Ge van Geldorp]

Hi Joel,

> From: Joel Holdsworth 
>
> How do you deal with this problem, because presumably not every test
> contributor has VMs ready to confirm the behaviour of 95, 98, ME, 2000,
> XP and Vista. Do I submit the patch as-is, in the knowlege that it will
> cause failures in old windows? Presumably not - so what do you suggest I
> do?

I have a whole bunch of VMs set up specifically for Wine testing, so feel
free to send me patches you think might behave differently on different
Windows versions. One caveat: I personally stopped caring about 9x when
Win2000 came out, so I'm not testing on those versions.

The good (but unsurprising) news is that your new tests pass on all my XP (4
different configurations) and Win2k3 (also 4 different configurations)
machines. I don't see failures on NT4, but perhaps the test was skipped
there?
The bad news is they failed on W2k, Vista and Win2k8. I've attached the test
reports for all failures.

Ge.



WVISTAADM.rpt
Description: Binary data


WNT4WSSP6.rpt
Description: Binary data


WNT4SRVSP6.rpt
Description: Binary data


W2KSRVSP4.rpt
Description: Binary data


W2KPROSP4.rpt
Description: Binary data


W2KASSP4.rpt
Description: Binary data


W2K8SE.rpt
Description: Binary data

RE: [2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Ge van Geldorp]

Hi Paul,

> From: Paul Vriens [mailto:paul.vriens.w...@gmail.com] 
>
> I've added these to the tests to show that Windows crashes. This is thus 
> a test (although not run) for behavior. It just means that Windows 
> doesn't check the input parameters thoroughly and so we shouldn't 
> either. It was trigerred by Coverity that reported a possible 
> dereference on a NULL parameter.

Sorry, I hadn't noticed that you changed something between my initial
version of these tests and my fixes this weekend. I'll put them back. I
still don't see the difference between this and the earlier case, but I've
given up on trying to find any consistency in Wine submissions anyway.

Ge.

RE: [2/2] secur32/tests: Fix GetUserNameEx() tests (try 2)

[Ge van Geldorp]

Hi Paul,

> From: Paul Vriens
> 
> Why did you remove:
>
> -if (0) /* Crashes on Windows */
> -rc = pGetUserNameExW(NameSamCompatible, NULL, NULL);
> -
>
> The point of these additions was to show/document that Windows crashes 
> when called like this.

Because Alexandre told me for an earlier patch to remove tests that crash on
Windows: http://www.winehq.org/pipermail/wine-devel/2009-January/072010.html

Ge.

RE: [2/2] secur32/tests: Fix GetUserNameEx() tests

[Ge van Geldorp]

> From: Austin English [mailto:austinengl...@gmail.com] 
>
> Shouldn't these be win_skip's?

Another day that wasn't wasted because I learnt something new :-). Thanks, 
resubmitted.

Ge.

RE: [2/3] secur32/tests: Add simple tests for GetUserNameExA/W() [try 2]

[Ge van Geldorp]

Hi Paul,

> From: Paul Vriens [mailto:paul.vriens.w...@gmail.com] 
> Ge van Geldorp wrote:
> > Try 2: Add buffer overflow tests
> > 
> > Changelog:
> >   secur32/tests: Add simple tests for GetUserNameExA/W()
> > 
> > ---
> >  dlls/secur32/tests/secur32.c |   86
> > ++
> >  1 files changed, 86 insertions(+), 0 deletions(-)
> > 
> > 
> Hi Ge,
>
> The tests show a big list of possible last errors. Are these for a 
> specific platform? If so, we should probably list that. Is the 
> ERROR_INVALID_PARAMETER for NameUnknown also for a specific platform?

I've now tested on all Windows platforms. Passing NameUnknown returns
ERROR_NONE_MAPPED if the current user is a local (non-domain) account and
ERROR_NO_SUCH_USER if the current user is a domain account.
The ERROR_NONE_MAPPED code is returned for all formats except
NameSamCompatible if the current user is a local account.

> The reason I'm asking is because W2K fails with ERROR_INVALID_PARAMETER 
> for NameDnsDomain and I'm looking for the cleanest way to add that (as 
> broken()).

Based on my new test runs, I came up with the following test:

  ok(rc ||
 (formats[i] == NameUnknown &&
  GetLastError() == ERROR_NO_SUCH_USER) ||
 GetLastError() == ERROR_NONE_MAPPED ||
 broken(formats[i] == NameDnsDomain &&
GetLastError() == ERROR_INVALID_PARAMETER),
 "GetUserNameExW(%d) failed: %d\n",
 formats[i], GetLastError());

This passes on all my Windows machines. Personally, I probably wouldn't have
used broken() for the W2K case but I'll follow your lead :-)

Ge.

RE: [2/3] secur32/tests: Add simple tests for GetUserNameExA/W() [try 2]

[Ge van Geldorp]

Hi Paul,

> From: Paul Vriens [mailto:paul.vriens.w...@gmail.com] 
> > Ge van Geldorp wrote:
> > Try 2: Add buffer overflow tests
> > 
> > Changelog:
> >   secur32/tests: Add simple tests for GetUserNameExA/W()
> > 
> > ---
> >  dlls/secur32/tests/secur32.c |   86
> > ++
> >  1 files changed, 86 insertions(+), 0 deletions(-)
> > 
> > 
> Hi Ge,
>
> The tests show a big list of possible last errors. Are these for a 
> specific platform? If so, we should probably list that. Is the 
> ERROR_INVALID_PARAMETER for NameUnknown also for a specific platform?
>
> The reason I'm asking is because W2K fails with ERROR_INVALID_PARAMETER 
> for NameDnsDomain and I'm looking for the cleanest way to add that (as 
> broken()).

Since GetUserNameEx() and GetComputerObjectName() are so closely related I
copied and modified the existing GetComputerObjectName() tests, that's where
the possible error list comes from. I didn't actually test this on each
platform.
Since I broke it, I feel responsible for fixing it, but I won't have time to
look at this until the weekend.

Ge.

Re: 'Interactive' tests on Windows 2003

[Ge van Geldorp]

From: "James Hawkins" 
> On Fri, Jun 13, 2008 at 8:40 AM, Francois Gouget  wrote:
> >
> > Someone asked to check which tests require interaction when running in
> > my Windows 2003 VM. However I cannot find the corresponding email (it
> > might have been on irc), so I'm sending this data to the list.
> >
> > So the only interaction that was needed was caused by this IE
> > security dialog:
> >
> >Content within this application coming from the Web site listed
> >below is being blocked by Internet Explorer Enhanced Security
> >Configuration.
> >
> >
> > Only three tests were impacted:
> >  * mshtml_test dom
> >   -> 1508 tests run but 0 failures happened when just closing the
> >  dialog.
> >
> >  * mshtml_test script  (about:security_mshtml_test.exe)
> >   -> 21 tests run and 13 failures if the dialog is just closed, and 0
> >  failures if mshtml_test is manually added to the 'allowed' list.
> >
> >  * urlmon_test misc
> >   -> 923 tests run but 0 failures if the dialog is just closed.
> >
> >
> > When run by winetest these tests just time out though.
> >
>
> Thanks Francois (I asked on IRC a while back).  I get these dialogs in
> 2k3 as well.  Do you think we should add a check for
> winetest_interactive before the calls that bring up the permission
> dialog?  If anyone else knows of a better way, let us know.

Actually, the interactive test is already there:

http://www.winehq.org/pipermail/wine-cvs/2009-January/051706.html
http://www.winehq.org/pipermail/wine-cvs/2009-January/051707.html

Ge.

Re: mshtml/tests: Fix tests on Windows

[Ge van Geldorp]

> From: Reece Dunn 
> 2009/4/1 Ge van Geldorp :
> > Modify load_state only in ReadyState_Changed callback.
> >
> > Changelog:
> > ?Fix tests on Windows
> >
> > ---
> > ?dlls/mshtml/tests/htmldoc.c | ? 28 +++-
> > ?1 files changed, 23 insertions(+), 5 deletions(-)
> > ?mode change 100644 => 100755 dlls/mshtml/tests/htmldoc.c
>
> Is there a reason for the mode change?
>
> - Reece

No, there isn't. Not sure how that happened, might be because I edited that
file using Visual Studio over Samba. I'll be more careful next time.

Ge.

urlmon ftp protocol failures on Windows

[Ge van Geldorp]

Hi Jacek,

I've been investigating the failures in the urlmon ftp protocol tests that
show up on quite a few WinXP and higher machines. It seems to me that there
is an inherent race condition in the test.
The main thread waits in line 1822 until IInternetProtocolSink_Switch has
been called. It will then call CHECK_CALLED(Switch) and SET_EXPECT(Switch).
However, nothing prevents a background thread from calling
IInternetProtocolSink_Switch again before SET_EXPECT(Switch) in the main
thread is complete. In fact, if I add a Sleep(100) between the
WaitForSingleObject() in line 1822 and the CHECK_CALLED() on the next line,
the failures that show up now and then on test.winehq.org become
reproducible every time.
I believe the attached patch fixes the problem, it breaks the current
assumption that there's a 1:1 relationship between
IInternetProtocolSinc_Switch() and IInternetProtocol_Read() calls. Before
shipping it off to wine-patches, I'd like your opinion, do you think this is
the correct solution?

Thanks, Ge.


protocol.patch
Description: Binary data

Re: wintrust(3/3): Fix a couple tests on a variety of systems

[Ge van Geldorp]

> From: Alexandre Julliard 
> 
> Paul Vriens  writes:
> 
> > I sent out an email earlier to ask if we should include more 
> > information in the header of the report. We could use that 
> > information, if this change will be accepted by AJ, to 
> > limit the reporting only to boxes were we have admin rights.
> >
> > As soon as all boxes have zero failures (yeah, right) we 
> > can remove that limit.
> 
> Yes, we will want to fix these tests eventually, since we 
> will some day support multi-user setups where normal users 
> won't have admin rights, and we will need tests to make sure 
> we fail the right way. But in the meantime we can certainly 
> block these reports and concentrate on the more meaningful failures.

I'll take that as a hint to switch my test machines to full admin. Should
result in quite a bit more green.

Ge.

RE: wintrust(2/3): Remove a couple tests that fail on a variety of systems (try 2)

[Ge van Geldorp]

> From: Paul Vriens [mailto:paul.vriens.w...@gmail.com] 
> 
> Paul Vriens wrote:
> > Juan Lang wrote:
> >> This removes the unused variables in the last version.
> >> --Juan
> >>
> >>
> >> 
> -
> >> ---
> >>
> >>
> > I think that these failures are also because of "not enough rights" 
> > (people running winetest as a normal user). So just removing them is 
> > probably not the best approach?
>
> Maybe it's not that there are not enough rights. The common 
> denominator that I see is that all reports/boxes that produce 
> these failures seems to be coming from one person (Ge). Maybe 
> worthwhile checking with him before just removing the tests?

All the failure cases on my machines are when running as non-Administrator.
I've set up a number of boxes deliberately that way, but I'm starting to
wonder if that is a good idea? For a whole bunch of tests (MSI comes to
mind) the only sensible approach is to skip the test when running as
non-Administrator, so it seems that I'm not actually adding new test runs in
a different environment as I originally wanted to do.

Gé.

user32:menu test

[Ge van Geldorp]

> From: Florian Köberle [mailto:flor...@fkoeberle.de] 
> 
> I ran the test 30 times using the same way you did without 
> any failure.I used a vm with 1 CPU assigned too. Installed
> is a Windows XP with SP2.
> 
> If I start the test twice so that two tests are running 
> parallel then I get test failures too, but:
> 
> Do we really have the requirement that tests must be able to 
> run in parallel?

Sorry about late reply, I've been away.
No, I don't think we have the requirement that tests must be able to run in
parallel. The only reason I brought that up is because that seems to be a
good way to make the failure consistently reproduceable.

Ge.

RE: First complete test run passing on a Windows platform

[Ge van Geldorp]

> From: Ge van Geldorp [mailto:g...@gse.nl] On Behalf Of 'Florian Köberle'

Sorry, I messed up the headers. The previous message containing:

> BTW, this causes failures also with my own fix applied (althoug only a
> single failure instead of a bunch), so my own fix wasn't correct either.

was sent by me, not by Florian.

Gé

RE: First complete test run passing on a Windows platform

[Ge van Geldorp]

Florian,

> From: Florian Köberle [mailto:flor...@fkoeberle.de] 
> How many cores do your processor have?
> I have a dual core which might make the difference.

I ran inside a VM that had 1 CPU assigned, so single core.

> How exactly do you execute the tests?
> I executed "../../../tools/runtest -q menu" about 20 times 
> without any failure.

I copied user32_crosstest.exe to C:\winetest, then created a small batch
file runtest.cmd:

@echo off
cd \winetest
user32_crosstest menu
pause

Put a shortcut to runtest.cmd on the desktop and then set the shortcut to
run Minimized (during my regular test runs the console is minimized too, so
I thought I'd mimic that as much as possible). Then double click the
shortcut, wait until the test is done, restore the console window and look
at the results. 

> Did my patch reduce the number of failures?
> I am asking as I wonder, if I should send the patch anyway to 
> wine-patches or not.

It's hard to say because it doesn't reproduce consistently. My feeling is
that it doesn't reduce the number of failures, but no hard data to back that
up.

> Does removing the Sleep(500) increase the fail rate?
> I tried to remove it but it had no effect for me at all.

No, I don't see an effect either.

I've done some more experimenting and can get the problem to reproduce more
consistently (like 95% of the time instead of 25%) by launching my shortcut
twice (so I have two test runs going at the same time). Then both of the
runs will usually show problems.

Gé.

RE: First complete test run passing on a Windows platform

[Ge van Geldorp]

Hi Florian,

> From: Florian Köberle [mailto:flor...@fkoeberle.de] 
> I think the main problem is that the thread might run 
> parallel with some other test code.
> 
> That is why I would suggest to wait for the thread to finish 
> at the end of the test.
> 
> I made a patch and attached it to this email. I can't 
> reproduce the failure on my system, so I can't test if the 
> patch helps. It would be cool, if someone could test, if my 
> patch fix the issue.

I applied your patch and ran the test about a dozen times, two runs had
failures. So I'm afraid your patch is not enough.

Gé.

RE: First complete test run passing on a Windows platform

[Ge van Geldorp]

> From: Reece Dunn [mailto:mscl...@googlemail.com] 
>
> Do you know why you need to do a SetForegroundWindow there to 
> make the tests succeed?
> Why is the input being sent to a different window?
> Why does it vary from platform-to-platform and machine-to-machine?

It's a timing issue. It doesn't just vary from machine-to-machine but even
from testrun-to-testrun (e.g. gvg-wxpprosp2 failed on 29 Jan but passed on
27 Jan). This is on VMs which get reset to a snapshot before running the
tests, so the machine starts each test run identically.

Gé.

RE: First complete test run passing on a Windows platform

[Ge van Geldorp]

> From: Reece Dunn 
> 
> The Wine tests are now passing on a Windows XP machine 
>
(http://test.winehq.org/data/4b27dfec939d131c9d7e09f97f14dfc7dabe8843/#group
_XP).

Actually, that was not the first time. On 20 Jan an XP machine passed:
http://test.winehq.org/data/e9d8c9f572998054b1f9c386ea81a3570c65f2d2/#group_
XP
And on 27 Jan a 2003 machine:
http://test.winehq.org/data/8f829034f3fe4da3e7adce2f4685e10ba2e2fe82/#group_
2003

> Two of the other four only have 2 or 3 failures.
> 
> The 2003 group has 3 machines with 1 failure each 
> (urlmon:protocol on one, user32:menu on the others).

I've sent a patch for the user32:menu failure a couple of weeks back but it
got blackholed
http://www.winehq.org/pipermail/wine-patches/2009-January/067483.html

I won't be submitting test results next week but don't worry, I'll be
back...

Ge.

RE: Ge van Geldorp : ole32/tests: Some Windows versions need the class to be registered.

[Ge van Geldorp]

> From: Rob Shearman [mailto:robertshear...@gmail.com] 
> > -error = RegCreateKeyEx(HKEY_CLASSES_ROOT, buffer, 0, NULL, 0, 
> > KEY_SET_VALUE, NULL, &hkey, 
> > +error = RegCreateKeyEx(HKEY_CURRENT_USER, buffer, 0, NULL, 0, 
> > KEY_SET_VALUE, NULL, &hkey, 
> 
> This appears to have caused some regressions in running the 
> tests on older versions of Windows:
> http://test.winehq.org/data/d0e77f8a47016c6f69e3a6b5a8bed8f620f1a8f1/nt4_cw-nt4sp6/ole32:marshal.html
> versus before:
> http://test.winehq.org/data/f63d950df75bdebbad3278790c6c9d11bfe1b226/nt4_cw-nt4sp6/ole32:marshal.html
> 
> While I can see how using HKEY_CURRENT_USER instead of 
> HKEY_CLASSES_ROOT can help developers using a standard user 
> rather than Admin on Windows, I don't see it as useful in 
> general as a lot of the tests emulate what installers do and 
> hence depend on having full access to the system.

Agreed. I'll submit a patch to revert this part of my change.

Ge.

Re: d3d8/tests: Make tests pass on W2K8

[Ge van Geldorp]

Hi Henri,

> From: Henri Verbeet
> 2009/1/26 Ge van Geldorp :
> > Changelog:
> >  d3d8/tests: Make tests pass on W2K8
> >
>
> >-ok(hr == D3D_OK || hr == D3DERR_INVALIDCALL,
"IDirect3D8_CreateDevice failed with %#08x\n", hr);
> >+ok(hr == D3D_OK || hr == D3DERR_INVALIDCALL || hr ==
D3DERR_NOTAVAILABLE, "IDirect3D8_CreateDevice failed with %#08x\n", hr);
>
> This looks more like something certain VMs would return than something
specific to a particular Windows version.

You're probably right, this was run on a VM. However, I don't see the same
failure on other Windows versions. And I guess the test should still pass,
even when it runs on a VM?

Gé.

RE: msi/tests: Make tests pass for Windows Installer 3.0 [2/3]

[Ge van Geldorp]

> From: Austin English [mailto:austinengl...@gmail.com] 
> 
> Just do as we do in other tests:
> ok(GetLastError == Error1 /* Win9X */ ||
>  Error2 /* Win2K */ ||
>  Error3 /* WinXP */...

Not possible like that in this case since the error checking is done by a 
helper function which takes the expected value as a parameter. Since the 
conscensus seems to be that we do need to check the values, I'll throw in a 
couple of new helper functions.

Gé.

Re: urlmon/tests: Fix protocol tests when proxy is configured

[Ge van Geldorp]

Hi Jacek,

> From: Jacek Caban 
> Subject: Re: urlmon/tests: Fix protocol tests when proxy is configured
> 
> Ge van Geldorp wrote:
> > There is a large amount of proxy configurations possible on 
> > Windows (WPAD, proxy.pac, fixed proxy values), which might behave 
> > differently on a second run of the protocol test compared to the
> > first run. These configurations influence when/how
> > IInternetProtocolSink methods are called. For example,
> > with a standard, direct connection you'll get the following 
> > ReportProgress notifications during the https test:
> > BINDSTATUS_FINDINGRESOURCE
> > BINDSTATUS_CONNECTING
> > BINDSTATUS_SENDINGREQUEST
> > BINDSTATUS_ACCEPTRANGES
> > BINDSTATUS_MIMETYPEAVAILABLE
> > However, when you run the protocol test for the second time 
> > on a machine with a fixed proxy server configured, you'll only
> > get these notifications:
> > BINDSTATUS_SENDINGREQUEST
> > BINDSTATUS_ACCEPTRANGES
> > BINDSTATUS_MIMETYPEAVAILABLE
> >
> > When a proxy is used, the BINDSTATUS_FINDINGRESOURCE and
> > BINDSTATUS_CONNECTING notifications report the hostname/ip 
> > address of the proxy server, not the end destination.
> 
> I can't see this behavior in tests results, where do you see 
> them? Which IE version is it?

This started when investigated the failures reported here:
http://test.winehq.org/data/8b10dd2a8daed738527f92aa89104c8647e4535c/2003_gv
g-w2k3r2sesp2/urlmon:protocol.html. This is a Windows 2003 R2 Standard
Edition SP2 machine with IE6. The status 32 shown there is
BINDSTATUS_PROXYDETECTING. It turns out that on this particular machine
"Automatically detect settings" (aka WPAD) was activated. The full sequence
of of ReportProtress notifications for the https test was:

BINDSTATUS_FINDINGRESOURCE
BINDSTATUS_CONNECTING
BINDSTATUS_PROXYDETECTING
BINDSTATUS_CONNECTING
BINDSTATUS_SENDINGREQUEST
BINDSTATUS_ACCEPTRANGES
BINDSTATUS_MIMETYPEAVAILABLE

I then started experimenting on that machine with various proxy settings. I
found that this sequence happens on the first run with WPAD configured but
no wpad.mydomain.mytld available. On a second run it seems the info that no
wpad was available was cached, I got "standard" behavior. If I configured a
wpad.mydomain.mytld domain the sequence would also revert back to "standard"
on the first run (whether there was a wpad.dat file available from that host
or not didn't seem to matter, there just needs to be a wpad.mydomain.mytld
DNS entry). When a proxy was indeed configured via WPAD, the
BINDSTATUS_FINDINGRESOURCE and BINDSTATUS_CONNECTING notifications reported
the hostname/ip address of the proxy server.
Next I experimented with "Use automatic configuration script" (aka
proxy.pac). I found that in that case the "standard" notification sequence
was used, but still with the proxy server hostname/ip address.
Finally I set a manual proxy server. In that case, the first time I run the
test I get the "standard" sequence, but on the second (and later) runs I see
the very minimal sequence mentioned in my original post.
So most of the behavior was seen in one-off experimentation. If you want, I
can set up some machines to report some of the behaviors to test.winehq.org.

> Also, you have to make sure that each SET_EXPECT is followed by 
> CHECK_CALLED (or CLEAR_CALLED if you want to ignore that function was 
> not called).

Ok, thanks, I'll take care of that.

Ge.

RE: msi/tests: Make tests pass for Windows Installer 3.0 [2/3]

[Ge van Geldorp]

Hi James,

> From: James Hawkins [mailto:trui...@gmail.com] 
> 
> On Sun, Jan 18, 2009 at 3:38 PM, Ge van Geldorp  wrote:
> > I see different estimated size values for Windows Installer 2.x and 
> > 3.0 compared to 3.1 and later.
> >
> 
> Please don't remove the check.  It is set, and that's what 
> we're testing.  If the value is different on different 
> platforms, then do like we do for other tests and add them to 
> the check.

Would it be ok then to check just for the existence of the value, not the
value itself? If you look at
http://test.winehq.org/data/1608cb99b5beb50e1b3f680c832be47944270730/xp_fg-w
inxp-sp1/msi:install.html and
http://test.winehq.org/data/1608cb99b5beb50e1b3f680c832be47944270730/xp_gvg-
wxpprosp2/msi:install.html you'll note that e.g. the test at install.c:2500
can return at least 3 different values, so I really don't think it makes
sense to check the value itself. I do see your point about checking for the
existence.

Ge.

RE: winetest: make the wrapper immune to Ctrl-C

[Ge van Geldorp]

> From: Dylan Smith [mailto:dylan.ah.sm...@gmail.com] 
> 
> On Fri, Jan 16, 2009 at 6:21 PM, Ge van Geldorp  wrote:
> >
> > The riched20:editor test seems to be sending Ctrl-C 
> > keystrokes to the wrong window on Windows. When running
> > "winetest -q" They end up in the winetest console window,
> > causing it to terminate. Judging from the lack of test
> > results, I'm not the only one experiencing this problem.
> 
> Thanks for noticing this, because I think I might have made 
> the change that caused this to happen.
> 
> I tracked down the source of the problem and I sent a patch 
> that seems to prevent this from happening:
> 
> richedit: Prevent sending Ctrl-C to console and ending tests.
> http://www.winehq.org/pipermail/wine-patches/2009-January/067836.html

Awesome, thanks.

> >
> > Changelog:
> >  winetest: make the wrapper immune to Ctrl-C
> >
> 
> If there is no problem with my patch, then do you think we 
> still need your patch as a preventative measure?

Seems to me that winetest should expect weird things to happen in the tests
it fires off and be as robust as possible in handling them. So yes, I do
think it would be a good idea to add the Ctrl-C handler to winetest anyway.
But in the end it's Alexandres call, not mine...

Ge.

RE: rpcrt4/tests: NdrVaryingArrayUnmarshall() is broken on Windows

[Ge van Geldorp]

> From: Paul Vriens [mailto:paul.vriens.w...@gmail.com] 
> 
> It looks like we've had exceptions on anything but XP and 
> W2K3 for a long time. 
> The exceptions (as reported on test.winehq.org) for XP and 
> W2K3 started happening after the move to use winetest.exe 
> from test.winehq.org.

I'm not sure which compilers are used. When I use VS2008, the test crashes
consistently on Windows, since in debug mode local vars are initialized with
a 0x pattern. MinGW 3.4.5 either initializes the var to zero, or
there happens to be a zero at the right location on the stack when the test
is executed. With MinGW cross-compiled binaries the test doesn't crash for
me on Windows.

Ge.

RE: rpcrt4/tests: NdrVaryingArrayUnmarshall() is broken on Windows

[Ge van Geldorp]

> From: Rob Shearman [mailto:robertshear...@gmail.com] 
> 
> 2009/1/12 Alexandre Julliard :
> > "Ge van Geldorp"  writes:
> > >
> > > Changelog:
> > >   NdrVaryingArrayUnmarshall() is broken on Windows
> >
> > If it's broken on Windows then there's no point in testing 
> > it, you can simply remove it.
> 
> There's value in the function working in Wine and testing the 
> function stops it being inadvertently broken (probably by 
> me). Microsoft may release a patch to fix it or fix it in a 
> more recent version of Windows.

That was my reasoning too. The test itself seems valid, it's just Windows 
that's broken. BTW, I did some more testing, initializing each of the fields in 
StubMsg in turn to zero. It's StubMsg->Offset that's causing the problems.
I've just submitted a patch that removes the test, Alexandre can pick whichever 
he prefers.

Ge.

WIDL and Windows rpcrt4

[Ge van Geldorp]

Hi Rob,

Is WIDL supposed to generate code that's compatible with Windows RPCRT4.DLL?
Because it appears this is currently not the case. When you look at
http://test.winehq.org you'll see that the rpcrt4:server test fails on
pretty much any Windows version.

I investigated a bit more, the failures are caused by an exception 0x6e6
("An internal error occurred in a remote procedure call (RPC)" thrown from
NdrPointerBufferSize() in server_c.c function get_name(). The
mingw-crosscompiled executable doesn't handle that exception which causes
the process to crash. Since this is in WIDL-generated code, I decided to
take a look at what MIDL generates for the same IDL file.

It turns out that MIDL treats the name_t struct as a simple structure, while
WIDL treats it as a complex structure. That made comparison a bit difficult.
Since WIDL never seems to generate simple structures, I tried to convince
MIDL to generate a complex structure too. After a bit of fiddling around
(I'm no IDL expert...) I came up with the following type, a mix of name_t
and sun_t:

typedef struct
{
  [string, size_is(size)] char *name;
  unsigned int size;

  [switch_is(s)] union
  {
[case(SUN_I)] int i;
[case(SUN_F1, SUN_F2)] float f;
[case(SUN_PI)] int *pi;
  } u;

  int s;
} complex_t;

After staring at the WIDL-generated and MIDL-generated code for a while
(which still differed quite a bit), I noticed something interesting: in the
type format string, "unsigned int size" gets encoded as FC_ULONG by WIDL,
but as FC_LONG by MIDL. I then went back to the server_c.c and server_s.c
files generated by WIDL for the tests and changed the type format strings
there (replaced the "unsigned int size" encoding from FC_ULONG by FC_LONG).
With this change, the get_name() test passes :-). It then crashes later on
with an access violation inside NdrVaryingArrayUnmarshall() in
get_5numbers(), but let's take the problems one at a time...

So it appears Windows RPCRT4.dll is not happy with FC_ULONG entries within a
complex structure. I added a bunch of other primitive types to my complex_t
structure to see how they are treated by MIDL. See attached complex.idl file
and the MIDL-generated midl_complex_c.c file (also attached WIDL-generated
widl_complex_c.c for completeness). It appears that MIDL simply drops the
"unsigned" from any base type, at least within complex structures. "unsigned
long" gets encoded as FC_LONG, "unsigned short" as FC_SHORT and "unsigned
small" as FC_SMALL.

Like I said, I'm no IDL expert. Does the stuff above make sense to you? Do
you think it would be a good idea to change WIDL to generate the same
encodings as MIDL?

Thanks, Gé.
/*** Autogenerated by WIDL 1.1.12 from complex.idl - Do not edit ***/
#include 
#ifdef _ALPHA_
#include 
#endif

#include "complex.h"

#ifndef DECLSPEC_HIDDEN
#define DECLSPEC_HIDDEN
#endif

#ifndef USE_COMPILER_EXCEPTIONS

#include "wine/exception.h"
#undef RpcTryExcept
#undef RpcExcept
#undef RpcEndExcept
#undef RpcTryFinally
#undef RpcFinally
#undef RpcEndFinally
#undef RpcExceptionCode
#undef RpcAbnormalTermination

struct __exception_frame;
typedef int (*__filter_func)(EXCEPTION_RECORD *, struct __exception_frame *);
typedef void (*__finally_func)(struct __exception_frame *);

#define __DECL_EXCEPTION_FRAME \
EXCEPTION_REGISTRATION_RECORD frame; \
__filter_func filter; \
__finally_funcfinally; \
sigjmp_bufjmp; \
DWORD code; \
unsigned char abnormal_termination; \
unsigned char filter_level; \
unsigned char finally_level;

struct __exception_frame
{
__DECL_EXCEPTION_FRAME
};

static DWORD __widl_exception_handler( EXCEPTION_RECORD *record,
   EXCEPTION_REGISTRATION_RECORD *frame,
   CONTEXT *context,
   EXCEPTION_REGISTRATION_RECORD 
**pdispatcher )
{
struct __exception_frame *exc_frame = (struct __exception_frame *)frame;

if (record->ExceptionFlags & (EH_UNWINDING | EH_EXIT_UNWIND | 
EH_NESTED_CALL))
{
if (exc_frame->finally_level && (record->ExceptionFlags & (EH_UNWINDING 
| EH_EXIT_UNWIND)))
{
exc_frame->abnormal_termination = 1;
exc_frame->finally( exc_frame );
}
return ExceptionContinueSearch;
}
exc_frame->code = record->ExceptionCode;
if (exc_frame->filter_level && exc_frame->filter( record, exc_frame ) == 
EXCEPTION_EXECUTE_HANDLER)
{
__wine_rtl_unwind( frame, record );
if (exc_frame->finally_level > exc_frame->filter_level)
{
exc_frame->abnormal_termination = 1;
exc_frame->finally( exc_frame );
__wine_pop_frame( frame );
}
exc_frame->filter_level = 0;
siglongjmp( exc_frame->jmp, 1 );
}
return ExceptionContinueSearch;
}

#define RpcTryExcept \
if 

Re: Race in thread shutdown in imm32?

[Ge van Geldorp]

> From: "Rob Shearman" <[EMAIL PROTECTED]>
> 
> I believe that Windows terminates all threads on process 
> exit

It does, see
http://blogs.msdn.com/oldnewthing/archive/2007/05/03/2383346.aspx

RE: Some feedfack for string {\style} on msi

[Ge van Geldorp]

> I am trying to fix the bug 13408 related with ActiveSync, but 
> i need some information of how and when appear the string 
> {\style} on msi.
> I need some specification or url for begin.

Try http://msdn.microsoft.com/en-us/library/aa367524(VS.85).aspx (or search
for "Adding Controls and Text").

RE: shell32: Support getting the shell folder path for users other than the current user and the default user [resend]

[Ge van Geldorp]

> From: Alexandre Julliard [mailto:[EMAIL PROTECTED] 
> 
> "Ge van Geldorp" <[EMAIL PROTECTED]> writes:
> 
> > +if (! CopySid(SidSize, Sid, UserInfo->User.Sid))
> > +{
> > +HeapFree(GetProcessHeap(), 0, Sid);
> > +Sid = NULL;
> > +}
> 
> You should return a string directly, there's no reason to 
> make a copy of the SID.

There's no way for the caller to free the memory if I return
UserInfo->User.Sid directly.

Ge van Geldorp.

RE: DIB Engine & GSoC

[Ge van Geldorp]

> If I understand it correctly, there was an idea floating 
> around that Ge van Geldorp had which was to auto generate the 
> needed DIB engine code for certain color depths and functions 
> so you would not have to implement the whole thing at once. 
> if I remember correctly he did a proof of concept 
> implementation for ReactOS by first creating the generic 
> interfaces that were needed and then generating the code for 
> the more simple color depths so as not to break all the 
> existing hacks they had. Maybe I am off base here and Ge will 
> comment as he lurks on wine-devel and I have cc'd him on this email.

This was specifically for BitBlt'ing. You have (potentially) 3 surfaces
involved in a BitBlt, the source surface, the destination surface and a
brush surface. For DIBs, the source and destination surface can be either 1,
4, 8, 16, 24 or 32 bits deep plus source and destination can have different
depths. The brush can be either a solid brush or a patterned brush. There
are 256 different ways to combine the surfaces (Raster Ops or ROPs).
All of these variables mean that although it's possible to write generic
code to handle everything that code is going to be littered with if's.
Meaning that generic code is going to be slow for simple operations like
filling a rectangle with a solid color. So you want to special-case the
most-often used cases and make them fast, while using the slow, generic code
as a fallback.

If you want to get the best performance, you need to write a lot of
almost-identical-but-slightly-different code. For example, in the innermost
loop you'll need to actually perform the ROP. But with 256 possible ROPs
that can be quite a number of if's to execute. And you're doing that inside
the innermost loop. To speed up things, I moved the ROP determination to the
outermost level. Based on the ROP one of 256 possible subroutines is called,
which in its innermost loop can just combine the bits in a way hardcoded for
that specific ROP (i.e. no more if's there's just e.g. "*Dest = *Src ^
*Dest" there). Actually, in the end I didn't use 256 subroutines but only
used subroutines for the most common ROPs (those with a symbol like PATCOPY)
and used a catch-all generic subroutine for the lesser used ROPs. All these
subroutines are almost the same, it's just the actual ROP code that's
different between them. And for some ROPs there's no source surface
involved, so for those ROPs you don't need to advance pointers into the
source surface when you're moving from row to row etc. (meaning you can't
just use a preprocessor macro, the changes between the subroutines are a bit
too complicated for that).

That's where the code generator came in. It generated all those slightly
different subroutines for the standard ROPs and a generic routine for the
rest. For an example, see http://oss.gse.nl/wine/dib8gen.c which contains
the generated bitblt routines for a 8-bit destination surface. Compare for
example the DIB_8BPP_BitBlt_Generic() routine near the top (the catch-all
one) with DIB_8BPP_BitBlt_PATCOPY() further down. The last one has very
tight inner loops (especially when BltInfo->PatternSurface is NULL, meaning
you're filling the destination rectangle with a solid color) compared to the
first one.

Of course, at the time I did measure performance to see if all this
optimization stuff indeed improved performance. And it did, dramatically
even. It's been a while, so I don't recall most of the performance numbers
anymore, but I do remember that I benchmarked some of the DIB BitBlt
operations and found that the generated code in ReactOS was about 3 times as
fast as the DIB code in Windows XP (on the same hardware of course).

I was actually quite proud of that code generator and the code it produced.
The DIB code generator is absolutely clean, MS doesn't ship anything like it
so it's simply impossible that it was created using reverse engineering. I
put the code generator under the LGPL, specifically so it could be used by
Wine if so desired. Note that the scope of this is limited to BitBlt's
though, it won't help when you need to draw a 3-pixel wide dash-dotted
ellips on your DIB surface...

Gé van Geldorp.

RE: compiling wine for amd64 on ubuntu dapper 6.06

[Ge van Geldorp]

> From: "Gerald Britton" <[EMAIL PROTECTED]>
> 
> 1. /configure can't find opengl, and produces these messages:
> 
> configure: WARNING: Wine will be build without OpenGL or 
> Direct3D support
> configure: WARNING: because something is wrong with the OpenGL setup:
> configure: WARNING: No OpenGL library found on this system.

I found the easiest way around this problem is to run configure using
--without-opengl. I doubt OpenGL is very useful at the moment for 64-bit
Wine anyway.

Ge van Geldorp.

RE: Patchwork (was Re: Governance revisited)

[Ge van Geldorp]

> From: Jakob Eriksson [mailto:[EMAIL PROTECTED] 
> 
> I have been watching this thread with keen interest.
> 
> Alexandre does not HAVE to respond to that patch, he can 
> silently ignore it just like he can now.
> 
> The only difference with Patchwork would be that after a 
> certain time with no comments and no commits, the patch would 
> be removed from the queue and the submitter would get an 
> email warning.

Actually, that's not how I intended things to work. The automatic removal
from the queue would only happen if the patch had a RFC status, i.e. if
action is expected from the patch submitter. If the patch is unopposed and
just waiting in the queue, it should stay there.
It's reasonable to tell a submitter "you seem to have lost interest in this
patch, it has been waiting on action from you for [a month, whatever] but
nothing has happened, therefore it will be removed from the queue". Sending
a warning message "your patch is going to be dropped without explanation" is
no improvement over the current situation.

Gé van Geldorp.

RE: Patchwork (was Re: Governance revisited)

[Ge van Geldorp]

Hello Mike, 

> From: Mike McCormack [mailto:[EMAIL PROTECTED] 
> 
> That sounds good, but it's not reasonable to put the 
> responsibility on Alexandre, as he has enough work already.

Unless you can read Alexandres mind, he's really the only one who can tell
what he didn't like about a certain patch. Hopefully he'll get help from
others to weed out obviously incorrect patches, but in the end it's his
decision.

> With a single maintainer system, costs to patch submitters 
> and authors are much less crucial to a working system than 
> costs to the single maintainer.

Which is why I made the remark about Alexandres special position. It doesn't
mean (at least I hope it doesn't mean) that costs to authors are not
important at all.

> We agree that encouraging more reviewers is a good thing, so 
> how about focusing on ways to get more people to review patches?

One doesn't preclude the other. I indeed agree it would be good to get more
people to review patches, but I also think that's not a complete solution.
The results of the review (either by peers, subsystem maintainers or
Alexandre) needs to be communicated back to the author. Focusing solely on
review doesn't solve the problem of patches getting lost either.

Ge van Geldorp.

Re: Patchwork (was Re: Governance revisited)

[Ge van Geldorp]

> From: Vitaliy Margolen <[EMAIL PROTECTED]>
> 
> So in a sense you will require some one to respond for any 
> incoming e-mail to wine-patches. And if no one does, 
> Alexandre don't get to see the status?

Not sure I understand what you mean. If no-one responds to the patch on
wine-devel the patch would remain queued and it would show up in Alexandres
list.

> What if he already 
> applied the patch? Now you slowing down what would have 
> worked just fine before.

If the patch is already applied it would be marked as such in the database.
When a message is sent to wine-devel after the fact, it won't get magically
un-applied.

> Ok now you making it dependant on an e-mail client. Yet 
> that's exactly what we are trying avoid with GIT.

Actually, my preference would be to do patch management via a web-based
interface. But that's just my preference, I assume a lot of other people
prefer the mailinglist method. So let's try to support both.

> Not really. You can't make some one to review something that 
> don't understand. Or if they are busy/on vacation/away from 
> PC/etc.

I'm not trying to change the review process, it would remain as-is. The only
addition would be a bot which could do some checks (e.g. does the patch
apply cleanly?). But that bot could be introduced anyway, even if Patchwork
isn't introduced, so I guess it's not an integral part of the proposed
system.

> So in the end you'll end up with either huge queue 
> that no one wants to touch. Or a "clean up" jobs that will 
> once in a while go and mark all patches as old and will 
> require authors to resubmit. How's that better then what we use now?

If no-one touches a patch, it would end up on Alexandres plate. He could
review the patch himself or ask a subsystem maintainer to review it. Again,
that's not different from the current system. If the queue of patches
waiting for commit grows without bounds, that's a clear indication that
Alexandre is overloaded and the project should find ways to remove some of
his workload. The same would happen without a patch management system, but
perhaps it wouldn't be as visible, so I'm chalking this one up as an added
benefit of a patch management system.
The system would also be self-cleaning. If a patch has been in RFC status
too long it would be deleted from the queue (with appropriate warning email
to the author)
http://www.winehq.org/pipermail/wine-devel/2006-September/051114.html

> When people well send out right hacks and expect some one to 
> tell then what they really should do. Current system allows 
> to no waste any time on such craft.

We seem to have fundamentally different views of the "peripheral" (non-core)
Wine developers. You seem to view them as potentially bad guys, going out of
their way to submit hacks and decrease the quality of Wine code as much as
possible, people you'd rather see leave. I see people willing to donate
their time to improve Wine. Sure, they make mistakes but at least they make
an effort.

> No, by requiring some one to respond you making them 
> responsible (at least until they respond). Of course 
> responses like "sucks" wouldn't be nice, so some one who does 
> understand the subject will have to spend their time to 
> understand the patch, write a review of the patch and send 
> it.

I don't get your point here. Isn't the purpose of reviewing to actually
review the patch?

> And you want that ASAP! Which means whenever patch 
> arrives in wine-patches some one (most likely more then one 
> person) will stop everything they are doing, and start 
> writing a review?

No, I don't want to change the review process. It can remain just as it is
now.

My objective is to improve Wine by maximizing the number of patches of
acceptable quality. In my opinion, this can be done by:

1) assuring no patches get lost
2) assuring an author gets informed about why his patch is not acceptable in
its current form so he can improve it.

Ge van Geldorp.

RE: Patchwork (was Re: Governance revisited)

[Ge van Geldorp]

> From: Mike McCormack [mailto:[EMAIL PROTECTED] 
> 
> Seems like that is a system that doesn't scale well at all, 
> as it requires Alexandre to specifically respond to each and 
> every patch.

No, it doesn't require that. It requires *someone* to respond, that could be
a fellow developer on wine-devel. A comment added via the web interface or a
message about the patch on wine-devel would set the status to RFC, meaning
the patch wouldn't show up in Alexandres list (or with a very low priority).
It would be the responsibility of the author to set the status back to New
if he thinks that's appropriate. Sorry if that wasn't clear from the message
you replied to, that message was explicitly aimed at the work Alexandre
does, there's more to the system than just that.

For the automatic status update to work, we would need to make an automatic
connection between wine-devel messages and the patch, could be done using
the In-Reply-To header or making sure each message sent out on wine-patches
has a unique ID in its subject, a reply to that message would (in most email
clients) copy the subject including the unique ID.

In the end, when the number of developers grows, the number of reviewers
grows too (every developer is a potential reviewer). Seems to scale pretty
well actually.

> It also seems like it encourages patch submitters to not 
> polish their patches themselves and just submit a higher 
> volume of low quality patches for Alexandre to review, since 
> the onus will then be on him to respond.

First of all, I don't see the encouragement and secondly, how does the
current system prevent that?

> The current system, which leaves the responsibility for the 
> patch with the submitter both scales better, and encourages 
> patch submitters to think about their patches more.

I'm not sure why you think responsibility for the patch would shift. It
would still be the authors responsibility to write acceptable code. The only
thing that would change is that the author gets feedback at the earliest
possible moment, be it from the bot, peer review or Alexandre.

> We should encourage more people 
> to participate in the patch review process, so that we have 
> more reviewers and a more scalable process.

Absolutely. The proposed system doesn't change the review process, it allows
peer review too. It just acts as a kind of safety net. Authority and
responsibility should go hand-in-hand. I hope it's clear that I don't have a
problem with Alexandre having the authority to make the final decision on
whether a patch goes in or not, I just believe that with that authority
comes the responsibility to inform the author if a patch isn't acceptable in
it's current form. Hopefully a fellow developer has already reviewed the
patch and told the author something is wrong but in the end we as developers
are not psychic and simply cannot know what Alexandre thinks about a patch.

> btw. Is there any reason that you can't request a review of 
> your patches, or report the problem that you're trying to fix 
> in bugzilla, as I suggested elsewhere?

How would that improve Alexandres productivity? As pointed out by Troy, it
just means he has to look at a patch twice before sending a reply. Not to
mention the time it costs the author. Shouldn't we be looking at the
productivity of everyone involved in Wine development and not just at
Alexandres productivity (although I acknowledge his special position)? I'm a
bit surprised (and, to be honest, also a little bit annoyed) about the low
value you seem to place on the time contributed by the developers.

Ge van Geldorp.

Re: Patchwork (was Re: Governance revisited)

[Ge van Geldorp]

> From: Alexandre Julliard <[EMAIL PROTECTED]>
> 
> If you expect 
> anything to happen, you'll need to make much more concrete 
> suggestions, and provide examples to make us understand how 
> what you are suggesting would work in practice

Fair enough. Some disclaimers upfront: I'm basically thinking out loud, I
have no idea what your typical workflow looks like. I'm also not restrained
by any knowledge of emacs and what can be done in its plugins, macro's,
user-defined commands or whatever they're called. But I know I could code
the proposal below in e.g Perl or C, so I guess it should be possible in
Lisp also. Who's going to code that is a different matter altogether, let's
first see if we can think of a solution which would work.

Let's assume a patch is submitted to wine-patches. A bot would receive it
and store it in a database (Patchwork already seems to do a fine job on
this). The bot could do some simple checks, like does it apply cleanly, no
C++ comments, whatever automatic checks we can come up with. If one of the
checks fails, the status would be set to RFC (Request For Change, I'm using
the Patchwork statuses here) and the author would receive an automatic email
explaining what's wrong, otherwise the status would be set to New. Note that
this should save you some time, weeding out "obviously" wrong patches.

On to your workflow. You'd issue a Meta-Ctrl-Shift-whatever emacs command.
Behind the scenes, this would send a request to the webserver which would
reply with a list of queued patches. This list could be sorted by a score.
Authors who consistently submit good patches would get a higher score than,
well, me. This score could be computed as the number of accepted patches
minus the number of rejected patches. A patch already reviewed (and
recommended) by a subsystem maintainer could get a real big score boost so
it ends up near the top of the list. Patches which have been in the queue
for a few days could get extra points. Patches with RFC status wouldn't be
shown at all or get a very low score. Anyway, you end up with a list of
patches on your screen with the least problematic near the top. This should
help ensure that these get through pretty quickly.

So you select a patch which is then retrieved from the server. It could be
automatically applied, a build started, whatever suits you best. Comments
made on it by other developers could be shown to you. After testing is
complete, you issue a "decision" command. You can decide to accept the
patch, that would send an "accept" message to the webserver and perhaps it
could be comitted to your git repo. If the patch needs more work, you'll be
prompted for a message. That message will be mailed to the author with a
copy to win-devel and it will be sent to the webserver which will change the
patch status to RFC. If you think the patch is beyond hope it can be
rejected outright (again with a message to author and wine-devel). Or you
can decide not to decide on it today and sleep on it for a night. Then the
patch will just show up in the list of pending patches again tomorrow, with
a little bit higher priority (since it has been in the queue longer).

Back to the list of pending patches, repeat.

> and how it would be different from what we are doing now.

It would make sure the author always receives some kind of feedback (either
from the bot, other developers or yourself) and would make sure patches
don't get lost (patches are automatically entered into the system and only
leave the system when the author withdraws them or when you make a final
decision).

Ge van Geldorp.

Re: Governance revisited (Wineconf report)

[Ge van Geldorp]

> From: Kai Blin
>
> Now, getting back to the patch submission process, you're talking about a 
> patch management system. How would that look like, in your opinion. We
were 
> discussing a couple of ideas, but in the end figured that most of those
would 
> slow down the submission speed of patches that are accepted.
>
> Our idea was to have patches be recieved by the subsytem maintainers, and 
> Alexandre would only apply patches he got from those guys. We voted that
one 
> down.
>
> Now I would be interested in your idea for such a system. I figure that 
> writing an emacs frontend won't be too troublesome if it's reasonably 
> designed, so let's ignore that for now.

A patch management system isn't exactly a new idea. It came up last year,
during the same type of discussion we're having now (and it will probably
come up next year when we have the same discussion again). See e.g.
http://www.winehq.org/pipermail/wine-devel/2005-September/039837.html and
note this sentence: "However, it became clear that the requirements were
fairly substantial (the tight emacs integration became our first clue :-/),
and that project got back burnered."

The users of such a patch management system would fall into 3 categories:
Developers/patch submitters, subsystem maintainers and Alexandre. I can take
a guess at what subsystem maintainers and Alexandre would want from such a
system, but of course I can only really speak as a patch submitter.
My primary requirements would be to be able to:
- Submit a patch
- Specify a subsystem
- Keep track of its progress
- Resubmit after receiving feedback
- Get an overview of patches in the queue
- Apply a queued patch to my working copy
- Provide feedback to others
- Withdraw a patch
In addition to the patch submitter requirements, subsystem maintainers would
probably like to be able to:
- Get an overview of patches relevant to their subsystem
- Recommend a patch for inclusion
In addition to all of the above, I guess Alexandre would need to be able to:
- Get an overview of recommended patches
- Commit a patch
- Reject a patch
- All from within emacs

Moving on to implementation, I see a set of database tables giving a patch
the following attributes:
- Submitter
- Short description
- Status (Queued, Waiting for developer, Recommended, Withdrawn, Committed,
Rejected)
- Subsystem
- Patch text (multiple versions, to allow resubmits and still keep a
history)
- Comments

Some status changes are restricted (e.g. only a subsystem maintainer would
be able to recommend a patch). Everyone would be able to submit a patch,
which would initially have Queued status. When feedback is provided, the
feedback provider (anyone) could change the status to Waiting for developer.
If a patch is in "Waiting for developer" status too long (say a month) it
could be automatically rejected. When "Waiting", the submitter could provide
a new version or respond with a comment, setting the status back to Queued
again. Or he could decide to Withdraw the patch. When a patch makes it into
the tree it will gain the most holy of all statuses, Committed. I envision
manually Rejecting a patch to be a pretty rare case, normally feedback would
be provided and the Waiting status would be set. Uncommittable patches would
then exit the system via the automatic clean up (in "Waiting" too long) or
via a Withdraw.

The heart of the system would be a couple of web pages. However, not
everyone likes web pages and the current system with mailing lists does have
its strong points too. So, there could be a two-way gateway between the web
system and the mailing lists. If a patch is submitted via the web, it could
be sent out via wine-patches too. If a patch comes in via wine-patches, it
could be added to the database automatically (when that doesn't succeed a
notification is sent to the author). Same for comments, they could be fed
from the web page to wine-devel.

If there is genuine interest to make this work I could put up a few mock
webpages to get a better idea of how it would work.

Gé van Geldorp.

Re: Governance revisited

[Ge van Geldorp]

> From: Steven Edwards <[EMAIL PROTECTED]>
> 
> Which is why we want to have the ambassadors project to help 
> new people in to wine. The thinking goes that if we have some
> people to help hold the hands of new developers and the
> developers that are defacto maintainers of a certain section
> of code will respond to patches as they seem them, this will
> free julliard from having to answer every single patch with a 
> reply.

You can have ambassadors and subsystem maintainers all you want, but in the
end it's still going to be Alexandre who decides if a patch goes in. That
means the end-responsibility of informing developers why a patch was
rejected needs to be with Alexandre. If an ambassador or subsystem
maintainer already explained it, fine, no need to create double work for
Alexandre, but if noone responded I'd still expect Alexandre to send a
notification.

Just for the record, my own policy on patch submission (and I really hope to
get back to working on Wine and submitting patches Real Soon Now :-)) is to
submit a patch once. If I get feedback I'll try to improve and resubmit, but
if it goes the black hole route I'm not going to beg for an explanation. If
the Wine community can't be bothered to provide feedback I can't be bothered
to resubmit. After all, I've already scratched my itch, the bug is already
fixed in my tree, it's the communities loss, not mine.

Ge van Geldorp

Re: Governance revisited (Wineconf report)

[Ge van Geldorp]

> From: Kai Blin <[EMAIL PROTECTED]>
> 
> On Saturday 23 September 2006 10:32, Scott Ritchie wrote:
> > Frankly, all we really need is for Alexandre to write a 10-second 
> > reply to wine-devel for each patch he rejects.
> 
> On WineConf, we decided against this. That would still slow 
> down the overall patch submission speed. Consider you have a 
> patch that's just fine, but before you sent that, I sent in 
> ten patches with C++ style comments. Alexandre would now have
> to reply to ten patches with "No C++ style comments" before
> processing your patch. Everybody reading wine-patches 
> could point out what was wrong with my patches.

You must be kidding. Even in your somewhat convoluted example, it would be
fine to receive just a single notification "No C++ style comments" and I
really wouldn't care if the notification came from Alexandre or from someone
else on wine-patches. The reality is that a lot of patches (I haven't kept a
precise count, but I estimate about a third of my own patches) are dropped
silently, without any feedback at all.

I have absolutely no problem with a strict patch acceptance policy, designed
to keep the Wine quality high. Alexandre is amazingly smart and when he
tells me why a certain patch was rejected I'll usually agree with him and
even if I don't agree, I have enough confidence in him to accept his
judgement. But having your patches dropped silently is pretty annoying. The
usual response from the core Wine developers is "well, just resubmit". I
fail to see how this helps Alexandres productivity (or my own for that
matter). If patches weren't dropped silently the sequence of events would
be:

1) Submit patch
2) Alexandre looks at patch
3) Alexandre writes 10-second rejection reply

With the resubmit:

1) Submit patch
2) Alexandre looks at patch
3) Wait a week
4) Resubmit patch
5) Alexandre looks at patch again
6) Alexandre writes 10-second rejection reply

Seems to me this incurs extra work for Alexandre (number 5).

The patch processes isn't very transparant. For example, I submitted a patch
on 20 June 2006 to dlls/kernel/heap.c (www.winehq.org seems to be
unreachable for me at the moment so I can't give a URL to the message) to
fix a problem with the heap on Win64. This patch was silently dropped. On
July 21 a change very similar to what I submitted was committed but not
attributed to me. Now, either the commit was not based on my patch (in which
case someone else spent another 2-3 days (it was one of those problems where
a chain of out-of-buffer memory writes take place and you have to work your
way up the chain to find the root cause) on tracking down an already known
problem for which a fix was already available) or it was based on my patch
(in which case my patch shouldn't have been dropped and I should have been
given credit).

Like I said before, I have a lot of respect for Alexandres technical
abilities. But when I read comments in the Wineconf report about git:
"Developers might not like it, but Alexandre does so it's a success" (did I
mention I dislike git also???) and the inability of the Wine community to
set up a patch management system (so patches won't disappear into the big
void) because Alexandre refuses to use it if it won't work in Emacs I'm
starting to wonder if people realise that the developers don't work for
Alexandre. He's a great Benovelent Dictator on technical issues, but in my
opinion only a Dictator on patch process management.

Ge van Geldorp.

RE: Win64 patch 3/6 (resend)

[Ge van Geldorp]

> From: Alexandre Julliard [mailto:[EMAIL PROTECTED] 
> 
> The best way would be to fix the few offending requests so 
> that we don't need to make every single request larger.

Not sure I understand what you mean. If I look at struct
send_message_request, it's current 64-bit layout is:

struct send_message_request
{
struct request_header
{
int  req;   /* 4 bytes */
/* 4 bytes padding */
size_t   request_size;  /* 8 bytes */
size_t   reply_size;/* 8 bytes */
} __header;
thread_id_t id; /* 4 bytes */
int type;   /* 4 bytes */
int flags;  /* 4 bytes */
/* 4 bytes padding */
user_handle_t   win;/* 8 bytes */
unsigned intmsg;/* 4 bytes */
/* 4 bytes padding */
unsigned long   wparam; /* 8 bytes */
unsigned long   lparam; /* 8 bytes */
int x;  /* 4 bytes */
int y;  /* 4 bytes */
unsigned inttime;   /* 4 bytes */
unsigned intinfo;   /* 4 bytes */
int timeout;/* 4 bytes */
/* 4 bytes padding */
void*   callback;   /* 8 bytes */
/* VARARG(data,bytes); */
};
Total 104 bytes.

Even if we rearrange the fields so minimum padding is required:

struct send_message_request
{
struct request_header
{
int  req;   /* 4 bytes */
/* 4 bytes padding */
size_t   request_size;  /* 8 bytes */
size_t   reply_size;/* 8 bytes */
} __header;
user_handle_t   win;/* 8 bytes */
unsigned long   wparam; /* 8 bytes */
unsigned long   lparam; /* 8 bytes */
void*   callback;   /* 8 bytes */
thread_id_t id; /* 4 bytes */
int type;   /* 4 bytes */
int flags;  /* 4 bytes */
unsigned intmsg;/* 4 bytes */
int x;  /* 4 bytes */
int y;  /* 4 bytes */
unsigned inttime;   /* 4 bytes */
unsigned intinfo;   /* 4 bytes */
int timeout;/* 4 bytes */
/* 4 bytes padding */
/* VARARG(data,bytes); */
};
Total bytes required: 96.

The current request_max_size is

struct request_max_size
{
int pad[16];
};

which is only 64 bytes. I'm not sure what you want changed in
request_send_message to make it fit in 64 bytes?

Ge van Geldorp.

RE: Win64 patch 3/6 (resend)

[Ge van Geldorp]

> From: Dmitry Timoshkov [mailto:[EMAIL PROTECTED] 
> 
> What exactly size is required then to avoid an assert?

This is the assert:

assert( sizeof(union generic_request) == sizeof(struct request_max_size) );

So the way the assert is written now "struct request_max_size" should have a
size exactly equal to the largest of any of the individual request_*
structures. Each of these request_* structures can contain a mix of 32-bit
and 64-bit (on Wine64) members.

> > Perhaps. It would become
> > 
> > struct request_max_size
> > {
> >long pad[10];
> >int pad1[6];
> > };
> > 
> > then, which seems just as arbitrary.
> 
> No, I meant to make it irrespective to 32 or 64-bit 
> compilation, i.e. look like:
> 
> struct request_max_size
> {
> long pad[16];
> };

That won't work, it will be too large for Wine64 (the largest request_* is
currently 104 bytes for Wine64, 64 bytes for Wine32) so the assert will
still fail.

Gé van Geldorp.

RE: Win64 patch 3/6 (resend)

[Ge van Geldorp]

> From: Dmitry Timoshkov [mailto:[EMAIL PROTECTED] 
> 
> "Ge van Geldorp" <[EMAIL PROTECTED]> wrote:
> 
> > --- a/include/wine/server_protocol.h
> > +++ b/include/wine/server_protocol.h
> > @@ -33,6 +33,9 @@ struct reply_header
> > struct request_max_size
> > {
> > int pad[16];
> > +#ifdef _WIN64
> > +int pad64[10];
> > +#endif
> > };
> 
> Why is this required? Is that due to asserts in server/request.c,
> open_master_socket() ?

Exactly.

> Is changing 'int' to 'long' a better fix?

Perhaps. It would become 

struct request_max_size
{
   long pad[10];
   int pad1[6];
};

then, which seems just as arbitrary.

> Also probably the fix should go in server/protocol.def instead.

Yes, you're right. I had missed the "automatically generated" comment at the
top of the include file. Will submit a new patch.

Gé van Geldorp.

RE: winnt32.exe on AMD64

[Ge van Geldorp]

> From: Boaz Harrosh
> 
>   Why not compile the all of Wine as PE with a GCC 
> cross-compiler (MinGW), But for the 3 low level DLL's. In 
> effect a ReactOS system and Makefiles (On a Linux kernel). 
> Sure we will maybe want an alternate system for 
> Builtin/Native, maybe different system folders for  builtin 
> dlls and executables vs. native. This will surely solve the 
> Above problem.

I hate to be so negative but at the moment this will not work. The reason is
simple, there's no 64-bit MinGW (yet?).

> Has any one (ReactOS?) tried such a system on Linux?

ReactOS could be cross-compiled, using MinGW, on Linux a few months ago. I
assume that's still possible.

> (And a philosophical Question: The "Sameness" of 32 bit 
> calling convention (and virtual tables) between PE and lets 
> call it GCC, was that a freak of nature? or is it just that 
> Unix people got there first this time and MS people must be 
> cocky? Or was it the Unix People who got cocky this time? ;-)

I was wondering about that too. I have no idea about the timeline, who was
first. It's conceivable that both parties worked on it at around the same
time.

G? van Geldorp.

RE: winnt32.exe on AMD64

[Ge van Geldorp]

> From: Troy Rollo [mailto:[EMAIL PROTECTED] 
> 
> These are complications rather than impenetrable barriers. 
> Wine->Wine (and Winelib App->Wine) calls would not need to go 
> through the thunks. Many of the issues involved have already 
> been dealt with for 16<->32 thunking and cross-process COM calls.
> 
> It's not ideal, but it should be possible.

It's an intriguing idea, but I can't see how it could work for even slightly
complex cases. Take the COM calls. When the COM object is constructed, you
have absolutely no idea whether its methods are going to be called from a
Wine or from a PE environment.
The difference with thunking and cross-process COM calls is that in those
cases the caller knows he's doing something special. Not so for in-proc COM
calls, it's just a function call.


Gé van Geldorp.

RE: winnt32.exe on AMD64

[Ge van Geldorp]

> From: "Nadgir, Ananth P" <[EMAIL PROTECTED]>
> 
> Does Wine now support executing x64 bit PE executable like 
> winnt32.exe under amd64 directory?

No, the biggest problem is that the 64-bit calling conventions are different
between PE executables and Linux executables. Some more info on the Wine64
wiki page http://wiki.winehq.org/Wine64

Ge van Geldorp.

RE: Win64 patch 3/9 (user32)

[Ge van Geldorp]

> From: Alexandre Julliard [mailto:[EMAIL PROTECTED] 
> 
> Ge van Geldorp <[EMAIL PROTECTED]> writes:
> 
> > Changelog:
> >   Ge van Geldorp <[EMAIL PROTECTED]>
> >   - Move handle indicator to upper 32 bits for Win64
> 
> Why do you need this?

I can't find the testcase which required this anymore. Will resubmit the
patch (with an explanation) when I hit the problem again.

Ge.

RE: include/msvcrt: Make sure size_t is properly defined for 64-bit.

[Ge van Geldorp]

> From: Alexandre Julliard [mailto:[EMAIL PROTECTED] 
> 
> We really shouldn't be including string.h here. Does this work for
> you?
> 
> diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
> index fc330ce..dcba4ae 100644
> --- a/dlls/msvcrt/msvcrt.h
> +++ b/dlls/msvcrt/msvcrt.h
> @@ -37,8 +37,6 @@
>  #ifndef __WINE_MSVCRT_H
>  #define __WINE_MSVCRT_H
>  
>  #include 
> -#include 
> -#include 
>  
>  #include "windef.h"
>  #include "winbase.h"

I did some more researching. The patch above does fix the compile-time
error. However, I still get compile-time warnings related to size_t (they
also occur with my original patch BTW):

(e.g. during compilation of dlls/msvcrt/tests/cpp.c)
In file included from ../../../include/wine/test.h:25,
 from cpp.c:30:
../../../include/msvcrt/stdlib.h:154: warning: conflicting types for
built-in function calloc
In file included from ../../../include/winnt.h:29,
 from ../../../include/windef.h:234,
 from ../../../include/wine/test.h:26,
 from cpp.c:30:
../../../include/msvcrt/string.h:68: warning: conflicting types for built-in
function memcmp

(plus a bunch of warnings for other built-in functions).
Although they are only warnings, they do point to a real problem: size_t is
defined as a 32-bit type but it should be a 64-bit type. size_t is defined
in a number of places in include/msvcrt:

direct.h
malloc.h
mbstring.h
search.h
stddef.h
stdio.h
stdlib.h
string.h
time.h
wchar.h

Most of these places use "typedef unsigned int size_t", while currently
stddef.h and string.h try to do the correct thing depending on whether
_WIN64 is defined, which doesn't work because that symbol is only defined if
either windef.h or basetsd.h were included before.
I'm not sure why the typedef is duplicated so many times. Wouldn't it be
better to define it only in stddef.h, #ifdef'ing it on __x86_64__ and then
include stddef.h from the other files?
Another option might be to add -D_WIN64 to EXTRACFLAGS in dlls/Makedll.rules
and programs/Makeprogs.rules. This would mimic the behaviour of MSVC better
(it has _WIN64 predefined).

Ge van Geldorp.

RE: include/msvcrt: Make sure size_t is properly defined for 64-bit.

[Ge van Geldorp]

> From: Ge van Geldorp [mailto:[EMAIL PROTECTED] 
> 
> Yes, this works fine, both with and without the change to 
> include/msvcrt/string.h.

Sorry, I only tested dlls/msvcrt. When doing a full rebuild I get errors,
will investigate and let you know.

Ge.

RE: include/msvcrt: Make sure size_t is properly defined for 64-bit.

[Ge van Geldorp]

> From: Alexandre Julliard
> 
> We really shouldn't be including string.h here. Does this work for
> you?
> 
> diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
> index fc330ce..dcba4ae 100644
> --- a/dlls/msvcrt/msvcrt.h
> +++ b/dlls/msvcrt/msvcrt.h
> @@ -37,8 +37,6 @@
>  #ifndef __WINE_MSVCRT_H
>  #define __WINE_MSVCRT_H
>  
>  #include 
> -#include 
> -#include 
>  
>  #include "windef.h"
>  #include "winbase.h"

Yes, this works fine, both with and without the change to
include/msvcrt/string.h.

Best regards, Ge van Geldorp.

Re: include/msvcrt: Make sure size_t is properly defined for 64-bit.

[Ge van Geldorp]

> diff --git a/include/msvcrt/string.h b/include/msvcrt/string.h
> index a821aa2..cc0b25e 100644
> --- a/include/msvcrt/string.h
> +++ b/include/msvcrt/string.h
> @@ -18,8 +18,18 @@ typedef unsigned short wchar_t;
>  #endif
>  #endif
>  
> +#ifndef _MSC_VER
> +# ifndef __int64
> +#  define __int64 long long
> +# endif
> +#endif
> +
>  #ifndef _SIZE_T_DEFINED
> +#ifdef _WIN64
> +typedef unsigned __int64 size_t;
> +#else
>  typedef unsigned int size_t;
> +#endif
>  #define _SIZE_T_DEFINED
>  #endif

Unfortunately, your changes to the submitted patch break it. The problem is
that at this point _WIN64 is not defined yet. It is defined in windef.h
which hasn't been included at this point. With your change, I still get the
following compile-time error during compilation of dlls/msvcrt/test/data.c:

In file included from data.c:21:
./../msvcrt.h:625: error: conflicting types for _strnset
../../../include/msvcrt/string.h:62: error: previous declaration of _strnset
was here

The patch I submitted fixes the problem by moving the include of "windef.h"
a bit up:

diff --git a/dlls/msvcrt/msvcrt.h b/dlls/msvcrt/msvcrt.h
index fc330ce..3da416d 100644
--- a/dlls/msvcrt/msvcrt.h
+++ b/dlls/msvcrt/msvcrt.h
@@ -36,11 +36,12 @@
 #ifndef __WINE_MSVCRT_H
 #define __WINE_MSVCRT_H

+#include "windef.h"
+
 #include 
 #include 
 #include 

-#include "windef.h"
 #include "winbase.h"
 #include "winerror.h"
 #include "winnls.h"

Gé van Geldorp.

RE: Win64 patch 1/5 (configure)

[Ge van Geldorp]

> From: Alexandre Julliard [mailto:[EMAIL PROTECTED] 
> 
> The appropriate address needs to be determined from the 
> standard Win64 address space layout. I don't have a Win64 box 
> to check what this looks like though.

Ok, I need some guidance on this. This is the usermode address space layout
when running a simple Win64 app:

0001 - 00011000 commit  private

0002 - 00021000 commit  private

0003 - 0012C000 reserve private
0012C000 - 0012E000 commit  private
0012E000 - 0013 commit  private stack
0013 - 00135000 commit  mapped  

0014 - 00141000 commit  mapped  

0015 - 00156000 commit  private process heap

0025 - 00254000 commit  mapped  
00254000 - 0026 reserve mapped 
0026 - 00276000 commit  mapped  

0028 - 002B4000 commit  mapped  

002C - 00301000 commit  mapped  

0031 - 00316000 commit  mapped  

0032 - 0033 commit  private
0033 - 00333000 commit  mapped 

0040 - 0048E000 commit  image   application

0049 - 004B1000 commit  private
004B1000 - 0059 reserve private

78D4 - 78EB2000 commit  image   kernel32.dll

78EC - 78ff9000 commit  image   ntdll.dll

7EFE - 7EFE9000 commit  mapped 
7EFE9000 - 7F0E reserve mapped 
7F0E - 7FFE reserve private
7FFE - 7FFE1000 commit  private shared user page
7FFE1000 - 7FFF reserve private

07FB - 07FD4000 commit  mapped

07FDD000 - 07FDF000 commit  private TEB
07FDF000 - 07FE commit  private PEB
07FE - 07FF reserve private

Some of the commit/mapped areas contain NLS files. As far as I can tell, the
TEB and PEB were moved from just-below-2GB to just-below-7TB, while the
shared user page and system DLLs like kernel32 and   ntdll were kept at
just-below-2GB.
I think 0x7bf00400 as relocation address for the Wine loader would work
fine. If you'd rather see a different load address or want me to investigate
something else please let me know.

Ge van Geldorp.

RE: Win64 patch 1/5 (configure)

[Ge van Geldorp]

> From: Alexandre Julliard [mailto:[EMAIL PROTECTED] 
> 
> > Changelog:
> >   Ge van Geldorp <[EMAIL PROTECTED]>
> >   - Allow relocation on x86_64
> 
> Is that really needed for Win64?  And if so, shouldn't it use 
> a more appropriate address?

The standard load address for 64-bit PE executables is the same as for
32-bit, 0x40. This is also the standard load address for 64-bit ELF
executables. If the Wine loader is not relocated, it won't be able to load
the majority of PE executables. Ofcourse, there's no problem with 64-bit
Winelib executables.
As to the more appropriate address, I'm not sure. I just wanted to get it
out of the way. I suppose it could be moved to somewhere around or above
0x7bf0.

Ge van Geldorp.

RE: Win64 patch 3/5 (winedbg)

[Ge van Geldorp]

> From: Eric Pouech [mailto:[EMAIL PROTECTED] 
> 
> this one is questionnable. Now that we have stackwalk64 
> implemented (I'm not saying it works flawlessly on 64bit 
> machines), it may be more interesting here to use the 64 bit 
> version of the call back

That's actually a separate issue. The problem I'm trying to solve here is
that the last parameter of ReadProcessMemory (which is the API that is
eventually called by pcs->process_io->read()) is defined as SIZE_T *, which
expands to a pointer to 8 bytes on Win64. The last parameter of the
ReadMemoryRoutine callback of StackWalk is PDWORD, pointer to 4 bytes. So
you can't pass the last parameter of the ReadMemoryRoutine callback on to
pcs->process_io->read(), you'd write 8 bytes to a 4 byte variable.

Unfortunately, switching to StackWalk64 doesn't solve this. The last
parameter of the ReadMemoryRoutine callback of StackWalk is still a pointer
to 4 bytes (PDWORD in Wine, LPDWORD in PSDK). Switching to StackWalk64 is
ofcourse a good idea, but we still need the patch as submitted.

Gé van Geldorp.

RE: Win64 patch 1/13

[Ge van Geldorp]

> From: Mike McCormack [mailto:[EMAIL PROTECTED] 
>
> > Except that dlls/ntdll/tests/generated.c was hand-modified.
> 
> That would explain the rather large diff that I saw when I 
> regenerated these tests :/

Sorry, I mixed up two files :( I have no evidence that
dlls/ntdll/tests/generated.c was hand-modified (dlls/oleaut32/oaidl_p.c,
another file for which I sent a patch, was). After updating to current git I
saw that you already changed tools/winapi/tests.dat, but it seems the tests
were not regenerated (so the dlls/*/tests/generated.c files are out of
sync).
Should I send a patch with the regenerated files or is it easier if
Alexandre just runs tools/winapi/winapi_test?

Gé van Geldorp.

RE: Win64 status

[Ge van Geldorp]

> From: Kuba Ober <[EMAIL PROTECTED]>
> 
> I'm currently hacking on gcc for an embedded target (Z8 
> Encore), but I wouldn't mind giving it a try while I'm 
> digging in the gcc tree.

Cool!

> I'd like to hear any suggestions as to how to tell the 
> compiler to use that calling convention (I'm not talking 
> about implementation, just how the compiler used would tell it).
> 
> I presume it'd need to be something that can be set globally 
> per each compiler invocation, as well as a per declaration override.
> 
> I'm thinking of "-msvc" (like -liberty ;) compiler option to 
> force a MSVC-compatible ABI, and for now that'd only apply to 
> 64 bit code (i.e. with -m64). Maybe in the future it could 
> enable something special for 32 bit code as well.
> 
> I'm all ears about the per-declaration override (a pragma? an 
> __attribute__?), and about alternatives to -msvc.

__attribute__ seems most logical to me. Perhaps __attribute__(__msvccall__)
(in the __attribute__(__stdcall__) tradition? An alternative to -msvc could
perhaps be -mrtd ("Alternate calling convention"). For i386 builds of gcc,
-mrtd makes stdcall the default calling convention. It is currently a noop
for x86_64. I kind of like your -msvc though :-)
For Wine the compiler option seems more important then the override.

> I'd be submitting a patch to mingw32 people as soon as it's 
> done, in addition to posting it here. Note that the only way 
> for me to test it would be to inspect the assembly output, as 
> I'm not running 64 bit environment here (even though I'm on a 
> 64 bit AMD processor). So it'd need testing from 64 bit 
> people here at least.

I'm volunteering. I could also give you ssh access to a 64 bit environment.

Ge van Geldorp.

CVS server

[Ge van Geldorp]

> From: Mike McCormack [mailto:[EMAIL PROTECTED] 
>
> I think you're a bit behind the times:
> 
>
http://cvs.winehq.org/cvsweb/wine/dlls/mapi32/mapi32_main.c.diff?r1=1.13&r2=
1.14
>
http://cvs.winehq.org/cvsweb/wine/dlls/user/user32.spec.diff?r1=1.114&r2=1.1
15

Indeed... Being on the European side of the ocean, I used the
rhlx01.fht-esslingen.de mirror. Seems it's no longer up-to-date. Apparently,
it has been removed from http://www.winehq.org/site/cvs too. Time to make
the switch to git I guess.

Gé.

RE: Win64 patch 1/13

[Ge van Geldorp]

> From: Mike McCormack [mailto:[EMAIL PROTECTED] 
> 
> Ge van Geldorp wrote:
> 
> > +SPEC_SRC32  = $(BASEMODULE).spec
> > +SPEC_SRC64  ?= $(SPEC_SRC32)
> 
> Not sure we want seperate spec files.  In any case, ?= 
> doesn't look portable.

I don't want them either, but I didn't see another option. 32-bit doesn't
export Get/SetWindowLongPtrA/W and GetSetClassLongPtrA/W (they are just
#defines in winuser.h), while 64-bit has to export them.
Is there a portable solution to set a Make variable only when it doesn't
have a value yet?

> I thought I already fixed these...  I don't think these will 
> apply to the current git tree.

Changes are against current CVS.

> > --- dlls/ntdll/tests/generated.c3 Jul 2005 11:23:30 
> - 1.23
> > +++ dlls/ntdll/tests/generated.c19 Jun 2006 17:20:17 -
> 
> You should be changing tools/winapi/winapi_test*, not the 
> generated files.

Except that dlls/ntdll/tests/generated.c was hand-modified.

Thanks for your comments, Gé.

Win64 status

[Ge van Geldorp]

With the Win64 patches I just submitted to wine-patches, I'm able to
successfully build Win64-enabled Wine and execute the following 64-bit
winelib (winelib64? wine64lib?) application:

#include "windows.h"

int WINAPI WinMain(HINSTANCE hinst, HINSTANCE hprev, LPSTR cmdline, int
cmdshow)
{
  DWORD Written;

  WriteFile(GetStdHandle(STD_OUTPUT_HANDLE), "Hello, world\r\n", 14,
&Written, NULL);

  return 0;
}

Unfortunately, compiling the above app as a true 64-bit HELLO64.EXE using
Microsoft Visual Studio and then executing it using Wine doesn't work. The
problem is that the calling convention for functions is different between
MSVC and GCC generated apps. Basically, MSVC puts the function arguments in
registers RCX, RDX, R8, R9 while GCC puts function arguments in registers
RDI, RSI, RDX, RCX, R8, R9. I guess we'll have to wait for true .EXE support
until someone teaches GCC the MSVC calling convention.

To prevent confusion: the "normal" 32-bit Wine version runs fine on x86_64
Linux. This is the version you'll want to use, it allows you to run 32-bit
Windows stuff. The remarks above are about running 64-bit Windows
executables which are virtually non-existant at the moment.

Gé van Geldorp.

RE: Unixfs as ShellFSFolder?

[Ge van Geldorp]

> From: Michael Jung
> 
> What do people think about getting rid of shfldr_fs.c in the 
> long run, in order to remove the redundancy? Does ReactOS use 
> wine's shell32.dll? I guess it would be a problem for them?

Yes, we do and yes, it would be a problem... We don't import the
shfldr_unixfs.c file because we have no use for it. I haven't really looked
at the unixfs stuff, but I'm wondering if it's perhaps possible to use an
inheritance mechanism? shfldr_fs and shfldr_unixfs could both inherit from a
common implementation, we can tack on some internal routines to the existing
vtable. Redundancy between the two can then be eliminated by extracting the
common code to the "base class" which would call through the vtable to the
internal routines.
If this is only for the benefit of ReactOS then of course I would be willing
to put in the work to make it happen.

Gé van Geldorp.

RE: Repackaging Mozilla ActiveX control to include MSVCP60.DLL?

[Ge van Geldorp]

> From: Boaz Harrosh
> 
> Hi! I was Just wondering. Is there at all a MinGW build 
> system for the "Mozilla ActiveX control"? I guess not so the 
> Question is:
> Any body knows what are the Main hurdles for that? Is that mainly COM
> TLB(s) related? or there are other problems?

I've built Firefox with MinGW, but never tried to rebuild the control (I
only repackaged it).

GvG

RE: Repackaging Mozilla ActiveX control to include MSVCP60.DLL?

[Ge van Geldorp]

> From: Dan Kegel
> 
> The Mozilla ActiveX control download feature is cool and all, but 
> until we repackage the sucker to include MSVCP60.DLL to fix
> http://bugs.winehq.org/show_bug.cgi?id=4064
> it's going to leave a lot of users scratching their heads as to why it 
> keeps asking where its files are.
> 
> I could have sworn I saw somebody post a note saying they had done 
> said repackaging, but I can't find it now...

http://www.winehq.org/pipermail/wine-patches/2005-December/022795.html
Instructions on how to repackage can be found at
svn://svn.reactos.org/trunk/tools/MozillaControl/

GvG

RE: STATUS_* definitions

[Ge van Geldorp]

> From: Vitaliy Margolen
> 
> Saturday, November 26, 2005, 10:34:22 AM, Ge van Geldorp wrote:
> > As proposed here:
> > http://www.winehq.org/pipermail/wine-devel/2005-November/042583.html
> > I didn't receive any replies, which I'll take as a "no objections".
> 
> > Changelog:
> >   Ge van Geldorp <[EMAIL PROTECTED]>
> >   - Match PSDK STATUS_* definitions
> 
> > Index: dlls/ntdll/cdrom.c
> > ===
> > RCS file: /home/wine/wine/dlls/ntdll/cdrom.c,v
> > retrieving revision 1.72
> > diff -u -r1.72 cdrom.c
> > --- dlls/ntdll/cdrom.c  26 Oct 2005 10:13:09 -  1.72
> > +++ dlls/ntdll/cdrom.c  26 Nov 2005 17:28:00 -
> I don't think we should touch ntdll in this way. As it should 
> use NTSTATUS (long int) instead of (DWORD).

I'm not sure I understand what you mean. I didn't touch the definition of
NTSTATUS in winternl.h, it is still long int. If the typecast changes in
ntstatus.h and winnt.h are made so they are compatible with the PSDK then
ntdll and server need to be modified in order to avoid compile-time
warnings.

Gé van Geldorp.

NTSTATUS values in ntstatus.h and winnt.h

[Ge van Geldorp]

The PSDK defines some NTSTATUS values in both ntstatus.h and in winnt.h,
guarded by "#ifndef WIN32_NO_STATUS". Unfortunately, the definitions in the
PSDK are not equivalent. In ntstatus.h:

#define STATUS_xxx ((NTSTATUS) 0x)

In winnt.h:

#define STATUS_xxx ((DWORD) 0x)

The Wine headers don't cast the literal to NTSTATUS resp DWORD. This means
it is ok to #include both winnt.h and ntstatus.h without #defining
WIN32_NO_STATUS using the Wine headers, but it is not ok to do so using PSDK
(or, in my case, ReactOS) headers. Would a patch adding the appropriate
casts in ntstatus.h/winnt.h and then fixing the resulting problems in Wine
by #defining WIN32_NO_STATUS where appropriate (an example is
dlls/netapi32/wksta.c) be acceptable?

Gé van Geldorp.

RE: wine / Linux kernel, TWAIN and STI: explained

[Ge van Geldorp]

> From: Saulius Krasuckas
> 
> * On Fri, 21 Oct 2005, Damjan Jovanovic wrote:
> > --- Saulius Krasuckas <[EMAIL PROTECTED]> wrote:
> > > 
> > > May the ReactOS code tree help you?
> > > http://reactos.geldorp.nl/dir_000323.html
> > 
> > Nice! Why isn't this part of wine already?
> 
> AFAIK, Some parts definitely are, but ROS folks just may be 
> busy enough to sync both trees in realtime. :-/ Steven 
> Edwards should have more clear idea about this, I guess. :-)

The ReactOS setupapi started out as a copy of the Wine setupapi. Recently, a
lot of stuff was added on the ReactOS side. Usually, we try to merge changes
back to Wine, but for various reasons this sometimes fails. For example,
some of our developers don't run Linux and therefore do not have the
possibility to test their changes under Wine too. There's also the feeling
that Wine should be responsible for their own codebase so it would be more
logical if a Wine developer was to take on the task of the backmerge.

Anyway, we effectively forked setupapi at Wine-20050524, the codebases
became too different to keep in sync easily. (We forked another component,
widl, at Wine-20050628). Of course, our changes are available to Wine, I'd
be happy to work with one of you guys to get the Wine/ReactOS versions back
in sync. BTW, I'm the guy who usually merges the Wine changes into the
ReactOS tree after a Wine release.

Ge van Geldorp.

RE: IDE disk geometry

[Ge van Geldorp]

> From: Vitaliy Margolen
> 
> So all you need to do 
> is to find where this information is stored on windows (it 
> has to be somewhere in registry

It's an IOCTL, so it is passed to the disk driver on Windows. Although it is
possible that the driver caches the info in the registry, I don't think
that's very likely (and it would be driver-dependent). That doesn't mean
that Wine can't store the info where it pleases (e.g. in the registry).

Ge van Geldorp.

ShellExecuteExW

[Ge van Geldorp]

This patch: http://www.winehq.org/pipermail/wine-cvs/2005-August/017595.html
broke execution of Control Panel items. The cpanel code would concatenate the
name of the .cpl file and the display name and pass that in the lpFile member
of the SHELLEXECUTEINFO struct. This is solved by passing .cpl in lpFile and
display name in lpParameters.
I ran into another problem: just before calling the execfunc(), any parameters
would be appended to wszApplicationName. Unfortunately, sei_tmp.lpFile points
to wszApplicationName at that point, so if the execfunc() fails (e.g. for a
.cpl file) sei_tmp.lpFile is messed up. Fixing this allowed one of the todo
tests to succeeed.
Since looking at/tracing through the ShellExecuteEx code makes me scream in
agony every time, I would appreciate a review of the patch below.

Ge van Geldorp.

Index: dlls/shell32/cpanelfolder.c
===
RCS file: /home/wine/wine/dlls/shell32/cpanelfolder.c,v
retrieving revision 1.20
diff -u -r1.20 cpanelfolder.c
--- dlls/shell32/cpanelfolder.c 27 Jul 2005 11:10:52 -  1.20
+++ dlls/shell32/cpanelfolder.c 8 Sep 2005 21:38:08 -
@@ -970,6 +970,7 @@
 SHELLEXECUTEINFOW sei_tmp;
 PIDLCPanelStruct* pcpanel;
 WCHAR path[MAX_PATH];
+WCHAR params[MAX_PATH];
 BOOL ret;
 int l;
 
@@ -990,12 +991,13 @@
 
 /* pass applet name to Control_RunDLL to distinguish between applets in 
one .cpl file */
 path[l++] = '"';
-path[l++] = ' ';
+path[l] = '\0';
 
-MultiByteToWideChar(CP_ACP, 0, pcpanel->szName+pcpanel->offsDispName, -1, 
path+l, MAX_PATH);
+MultiByteToWideChar(CP_ACP, 0, pcpanel->szName+pcpanel->offsDispName, -1, 
params, MAX_PATH);
 
 memcpy(&sei_tmp, psei, sizeof(sei_tmp));
 sei_tmp.lpFile = path;
+sei_tmp.lpParameters = params;
 sei_tmp.fMask &= ~SEE_MASK_INVOKEIDLIST;
 sei_tmp.lpVerb = wCplopen;
 
Index: dlls/shell32/shlexec.c
===
RCS file: /home/wine/wine/dlls/shell32/shlexec.c,v
retrieving revision 1.71
diff -u -r1.71 shlexec.c
--- dlls/shell32/shlexec.c  8 Sep 2005 18:54:52 -   1.71
+++ dlls/shell32/shlexec.c  8 Sep 2005 21:38:09 -
@@ -1263,9 +1263,10 @@
 
 lpFile = wfileName;
 
+strcpyW(wcmd, wszApplicationName);
 if (sei_tmp.lpParameters[0]) {
-strcatW(wszApplicationName, wSpace);
-strcatW(wszApplicationName, wszParameters);
+strcatW(wcmd, wSpace);
+strcatW(wcmd, wszParameters);
 }
 
 /* We set the default to open, and that should generally work.
@@ -1273,7 +1274,7 @@
 if (!sei_tmp.lpVerb)
 sei_tmp.lpVerb = wszOpen;
 
-retval = execfunc(wszApplicationName, NULL, FALSE, &sei_tmp, sei);
+retval = execfunc(wcmd, NULL, FALSE, &sei_tmp, sei);
 if (retval > 32)
 return TRUE;
 
Index: dlls/shell32/tests/shlexec.c
===
RCS file: /home/wine/wine/dlls/shell32/tests/shlexec.c,v
retrieving revision 1.5
diff -u -r1.5 shlexec.c
--- dlls/shell32/tests/shlexec.c23 Aug 2005 09:37:30 -  1.5
+++ dlls/shell32/tests/shlexec.c8 Sep 2005 21:38:09 -
@@ -704,9 +704,7 @@
 okChildInt("argcA", 5);
 okChildString("argvA3", "Open");
 sprintf(filename, "%s\\test file.shlexec", tmpdir);
-todo_wine {
 okChildPath("argvA4", filename);
-}
 
 sprintf(filename, "%s\\test_shortcut_exe.lnk", tmpdir);
 rc=shell_execute_ex(SEE_MASK_NOZONECHECKS, NULL, filename, NULL, NULL);

RE: wine/dlls/shell32 shresdef.h shres.rc shfldr_u ...

[Ge van Geldorp]

Michael, 

> I'm aware of this problem (though I didn't know about the 
> exact numbers). 
> There was a short discussion on wine-devel about it:
> 
> http://www.winehq.org/pipermail/wine-devel/2005-August/039452.html

I actually read the discussion but didn't make the connection when looking
at this problem.

> The problem is, that we currently can't read icons via 
> PrivateExtractIcons from wine's ELF dlls. I would guess that 
> for ReactOS, you don't need the hack'ish icon cache 
> pre-initialization at all, since you do have real PE dlls?

Yes, you're right on both counts, we do have real PE dlls and it turns out
we don't actually need the pre-init. Thanks for solving my problem :-)

> As was pointed out by Alexandre, the correct fix for wine 
> would probably be to try to load the given dll via 
> LoadModule, if all other methods fail in PrivateExtractIcon. 
> Unfortunetaly, I don't have time to work on this at the 
> moment, and probably won't have for the next months.

To fix the memory waste, maybe Wine could stop substituting icon with index
0 for non-existent icons in SIC_Initialize? PidlToSicIndex() will try to
find the icon and when it doesn't exist will return index 0 anyway.
This doesn't solve the general problem of loading resources from Wine dlls
ofcourse.

Gé van Geldorp.

RE: BUG[1481] : unimplemented function msvcrt.dll._mbsbtype

[Ge van Geldorp]

> From: Jesse Allen
> 
> Quick question.  Does React OS or DJGPP include a portable 
> implementation of printf in msvcrt?  I tried looking using 
> your source browser but I could not tell where it is actually located.

It's in lib/crt/stdio
(http://svn.reactos.com/viewcvs/trunk/reactos/lib/crt/stdio/)

Gé van Geldorp.

RE: BUG[1481] : unimplemented function msvcrt.dll._mbsbtype

[Ge van Geldorp]

> From: Vijay Kiran Kamuju
> 
> can we use the reactos implementation of the _mbsbtype?
> http://reactos.geldorp.nl/dc/d8d/mbbtype_8c-source.html
> 
> Any ReactOS guys, please respond regarding the licensing of the above 
> code?

Most of the code in our C Runtime originates from the DJGPP compiler
package. I've checked, the DJGPP libc is LGPL. So I think using this in Wine
should be no problem.

Gé van Geldorp.

RE: calling *W functions in wt (Was: dlls/shell32/shfldr_desktop.c)

[Ge van Geldorp]

> From: Saulius Krasuckas
> 
> Hello Ge and especially unicoders: 
> Alexandre, Dimi, Dmitry, Rob, Shachar, Troy and others.
> 
> I am not sure of how should be file operations implemented in 
> winetest:
> 
> *FileW and *DirectoryW functions fail on every win9x box as 
> they are unimplemented here. They succeed only when app is 
> linked to MS Layer for Unicode (MSLU) and MSLU is installed. [1]

Yeah, I see your point. I just followed the existing stuff in there, which
was all Unicode. For routines which implement the Ansi function by calling
the Unicode function with appropriate conversions, I guess it would be
better to test the the Ansi function, since this implicitly tests the
Unicode function too. Then again, this would make the test depend on
internals of the functions, not good. So the only solution seems to be to
test functions which have Ansi and Unicode implementations both ways (which
is kind of boring to write) on NT and skip the Unicode tests on Win9x.
Difficult stuff. I guess it's not an option to drop Win9x compatibility in
Wine .

Ge van Geldorp.

RE: dlls/shell32/pidl.c

[Ge van Geldorp]

> From: Michael Jung
> 
> SHBindToParent does not allocate a 
> new PIDL for pidlLast, but returns a pointer to right 
> location in pidl. This means you should not free it. 
> There's still the problem that the shell folder isn't 
> released in failure cases.
> 
> Sorry, I didn't realize this.

I feel a bit silly for not catching it either. MSDN is very clear about
this. I've resubmitted a patch which only releases the interface pointer.

Gé van Geldorp.

SHGetPathFromIDList problem

[Ge van Geldorp]

Since this patch: http://www.winehq.org/hypermail/wine-cvs/2005/06/0338.html
SHGetPathFromIDList() seems broken. The problem is that files stored in the
Desktop directory don't get the path to the Desktop directory prepended.
I've tried to fix it in the attached patch and added a testcase, but the shell
COM interfaces easily confuse me, so perhaps someone with a better understanding
of this stuff should look it over before I submit it to wine-patches.

Ge van Geldorp.

Index: dlls/shell32/pidl.c
===
RCS file: /home/wine/wine/dlls/shell32/pidl.c,v
retrieving revision 1.133
diff -u -r1.133 pidl.c
--- dlls/shell32/pidl.c 8 Jul 2005 14:18:32 -   1.133
+++ dlls/shell32/pidl.c 12 Aug 2005 15:04:05 -
@@ -1242,6 +1242,7 @@
 BOOL WINAPI SHGetPathFromIDListW(LPCITEMIDLIST pidl, LPWSTR pszPath)
 {
 HRESULT hr;
+BOOL result;
 LPCITEMIDLIST pidlLast;
 LPSHELLFOLDER psfFolder;
 DWORD dwAttributes;
@@ -1258,13 +1259,34 @@
 
 dwAttributes = SFGAO_FILESYSTEM;
 hr = IShellFolder_GetAttributesOf(psfFolder, 1, &pidlLast, &dwAttributes);
-if (FAILED(hr) || !(dwAttributes & SFGAO_FILESYSTEM)) return FALSE;
+if (FAILED(hr) || !(dwAttributes & SFGAO_FILESYSTEM)) {
+IShellFolder_Release(psfFolder);
+ILFree((LPITEMIDLIST) pidlLast);
+return FALSE;
+}
 
 hr = IShellFolder_GetDisplayNameOf(psfFolder, pidlLast, SHGDN_FORPARSING, 
&strret);
-if (FAILED(hr)) return FALSE;
-
-hr = StrRetToBufW(&strret, pidlLast, pszPath, MAX_PATH);
 IShellFolder_Release(psfFolder);
+if (FAILED(hr)) {
+ILFree((LPITEMIDLIST) pidlLast);
+return FALSE;
+}
+
+/* Check for a PIDL rooted at desktop level */
+if (_ILIsValue(pidl) || _ILIsFolder(pidl)) {
+result = SHGetSpecialFolderPathW(NULL, pszPath, 
CSIDL_DESKTOPDIRECTORY, FALSE);
+if (! result) {
+ILFree((LPITEMIDLIST) pidlLast);
+return FALSE;
+}
+PathAddBackslashW(pszPath);
+} else {
+pszPath[0] = '\0';
+}
+
+hr = StrRetToBufW(&strret, pidlLast, pszPath + lstrlenW(pszPath),
+  MAX_PATH - lstrlenW(pszPath));
+ILFree((LPITEMIDLIST) pidlLast);
 
 TRACE_(shell)("-- %s, 0x%08lx\n",debugstr_w(pszPath), hr);
 return SUCCEEDED(hr);
Index: dlls/shell32/tests/shlfolder.c
===
RCS file: /home/wine/wine/dlls/shell32/tests/shlfolder.c,v
retrieving revision 1.29
diff -u -r1.29 shlfolder.c
--- dlls/shell32/tests/shlfolder.c  12 Aug 2005 10:33:37 -  1.29
+++ dlls/shell32/tests/shlfolder.c  12 Aug 2005 15:04:06 -
@@ -584,6 +584,11 @@
 WCHAR wszMyComputer[] = { 
 
':',':','{','2','0','D','0','4','F','E','0','-','3','A','E','A','-','1','0','6','9','-',
 
'A','2','D','8','-','0','8','0','0','2','B','3','0','3','0','9','D','}',0 };
+WCHAR wszFileName[MAX_PATH];
+LPITEMIDLIST pidlTestFile;
+HANDLE hTestFile;
+static WCHAR wszTestFile[] = {
+'w','i','n','e','t','e','s','t','.','f','o','o',0 };
 
 if(!pSHGetSpecialFolderPathW) return;
 
@@ -604,15 +609,52 @@
 
 hr = IShellFolder_ParseDisplayName(psfDesktop, NULL, NULL, wszMyComputer, 
NULL, &pidlMyComputer, NULL);
 ok (SUCCEEDED(hr), "Desktop's ParseDisplayName failed to parse 
MyComputer's CLSID! hr = %08lx\n", hr);
-IShellFolder_Release(psfDesktop);
-if (FAILED(hr)) return;
+if (FAILED(hr)) {
+IShellFolder_Release(psfDesktop);
+return;
+}
 
 SetLastError(0xdeadbeef);
 result = SHGetPathFromIDListW(pidlMyComputer, wszPath);
 ok (!result, "SHGetPathFromIDList succeeded where it shouldn't!\n");
 ok (GetLastError()==0xdeadbeef, "SHGetPathFromIDList shouldn't set last 
error! Last error: %08lx\n", GetLastError());
+if (result) {
+IShellFolder_Release(psfDesktop);
+return;
+}
 
 IMalloc_Free(ppM, pidlMyComputer);
+
+result = pSHGetSpecialFolderPathW(NULL, wszFileName, 
CSIDL_DESKTOPDIRECTORY, FALSE);
+ok(result, "SHGetSpecialFolderPathW failed! Last error: %08lx\n", 
GetLastError());
+if (!result) {
+IShellFolder_Release(psfDesktop);
+return;
+}
+PathAddBackslashW(wszFileName);
+lstrcatW(wsz

RE: LoadImage & 32bpp bitmaps

[Ge van Geldorp]

> From: Kevin Koltzau
> 
> The solution IMO is to use a real DIB engine

Since ReactOS never had the benefit of being able to use X11 functions,
we've already done a lot of work on a DIB engine. As with just about
everything in ReactOS, it is not complete yet. However, often-used stuff
like BitBlt is fully supported, just don't expect Bezier curves drawn with a
3 pixel wide pen using a PATPAINT rop to work.

Recently we've added a code generator which generates pretty fast code for
BitBlt operations. At the moment, it only works for 16bpp destinations, but
I expect to add the other depths very soon. Timing tests show that this code
is significantly faster than the XP non-hardware-accellerated code.

It will take some effort to port this to Wine, since the internal
datastructures used are different (we're bound by the DDK engine types like
SURFOBJ). If there is interest in this from the Wine side, I'd be happy to
provide assistance.

Ge van Geldorp.

RE: tools/widl

[Ge van Geldorp]

I'm wondering what happened to the reworked widl patch at
http://www.winehq.org/hypermail/wine-patches/2005/04/0371.html? It hasn't
been applied yet, does that mean there's no interest on the Wine side and I
can drop it from my TODO list?

Best regards, Ge van Geldorp.

RE: tools/widl

[Ge van Geldorp]

> From: Alexandre Julliard
> 
> You should add some sort of get_fc_name() function instead of 
> duplicating a big switch like that every time you want to 
> print a name.

This is a big problem for me. Eric Kohl (not me) did a lot of work on WIDL
in the ReactOS tree, there's about 20 patches to follow after this one. If
this first patch is not acceptable and needs to be rewritten, it means I can
no longer simply pull following patches from our SVN repository. Since I
didn't write the original code in the first place, having to do patch
generation manually will a) significantly increase the chance of introducing
bugs and b) take too much of my time.
Maybe we can try to find a solution at WineConf?

Ge van Geldorp.

RE: dlls/shell32/shelllink.c

[Ge van Geldorp]

> From: Mike McCormack [mailto:[EMAIL PROTECTED] 
> 
> Ge van Geldorp wrote:
> > I have some shortcuts (created in Windows 2000) which only have 24 
> > bytes of LOCATION_INFO, i.e. they are missing the dwFinalPathOfs 
> > member. Also lots of shortcuts seem to have another chunk 
> > at the end of the stream, I get a lot of "Last word was not
> > zero" messages.
> 
> Could you send me a shortcut like that, or even modify winedump (lnk 
> mode) to dump out the right thing in that case?

Sure, will take me a day or two though, since I'm way behind on some other
stuff.

Ge van Geldorp.

wpp

[Ge van Geldorp]

wpp will currently silently drop non-preprocessor stuff (typedefs, function
prototypes etc.) when #including a .c or .h file. However, if you preprocess
e.g. a .rc file externally and then feed it to wrc (which in turn will feed
it through wpp) the non-preprocessor stuff isn't removed. The patch below
changes that, by looking at the filename on the #line directive. If it ends in
.c or .h, no output is generated (but the file is still parsed for preproc
statements ofcourse).
We need this for ReactOS, it doesn't hurt Wine (yes, I've tested it ) and
including it in Wine would make my monthly job of merging Wine and ReactOS a
tad easier. If there are no objections I'll submit to wine-patches in a few
days.
We also have another change in wpp, which adds support for #include_next (can't
take credit for that, Filip Navara wrote it). Since you can't use that
#include_next in Wine anyway (not every compiler supports it) I'm not going
to bother creating and testing a Wine patch for it. If I misjudged and there
is interest in this from the Wine side just let me know.

Ge van Geldorp.

Index: libs/wpp/ppl.l
===
RCS file: /home/wine/wine/libs/wpp/ppl.l,v
retrieving revision 1.4
diff -u -r1.4 ppl.l
--- libs/wpp/ppl.l  21 Oct 2003 23:57:25 -  1.4
+++ libs/wpp/ppl.l  20 Feb 2005 15:12:10 -
@@ -232,6 +232,7 @@
 static void newline(int);
 static int make_number(int radix, YYSTYPE *val, const char *str, int len);
 static void put_buffer(const char *s, int len);
+static int is_c_h_include(char *fname, int quoted);
 /* Buffer management */
 static void push_buffer(pp_entry_t *ppp, char *filename, char *incname, int 
pop);
 static bufferstackentry_t *pop_buffer(void);
@@ -551,11 +552,15 @@
case pp_define:
case pp_mbody:
case pp_inc:
-   case pp_line:
case RCINCL:
if (yy_current_state()==RCINCL) yy_pop_state();
pplval.cptr = get_string();
return tDQSTRING;
+   case pp_line:
+   pplval.cptr = get_string();
+   if (is_c_h_include(pplval.cptr, 1)) pass_data=0;
+   else pass_data=1;
+   return tDQSTRING;
default:
put_string();
}
@@ -1404,12 +1409,12 @@
  * Include management
  *-
  */
-static int is_c_h_include(char *fname)
+static int is_c_h_include(char *fname, int quoted)
 {
int sl=strlen(fname);
-   if (sl < 2) return 0;
-   if ((toupper(fname[sl-1])!='H') && (toupper(fname[sl-1])!='C')) return 
0;
-if (fname[sl-2]!='.') return 0;
+   if (sl < 2 + 2 * quoted) return 0;
+   if ((toupper(fname[sl-1-quoted])!='H') && 
(toupper(fname[sl-1-quoted])!='C')) return 0;
+if (fname[sl-2-quoted]!='.') return 0;
return 1;
 }
 
@@ -1448,7 +1453,7 @@
pp_incl_state.seen_junk = 0;
pp_incl_state.state = 0;
pp_incl_state.ppp = NULL;
-   if (is_c_h_include(newpath)) pass_data=0;
+   if (is_c_h_include(newpath, 0)) pass_data=0;
else pass_data=1;
 
if(pp_status.debug)

RE: Microsoft genuine downloads looking for wine

[Ge van Geldorp]

> From: Ivan Leo Puoti
>
> Interestingly if you run the validation program on wine,
> and the version of windows you're emulating is prior to
> 2000 or is windows server 20003, you get a message saying
> a validation code couldn't be found, because of technical
> difficulties or because you're running an unsupported
> operating system.
> If you set winver to win2000, you'll get a validation code
> that doesn't work, this may be a bug in wine, or in the
> validation program.

When I run the validation program on my genuine Win2k system, I get the
message saying a validation code couldn't be found because of technical
difficulties or because I'm running an unsupported operating system.
When using IE and thus the ActiveX control there is no problem and my
Windows is recognized as genuine.
Looks to me the standalone validation program is seriously broken

Gé van Geldorp.

RE: Summary of MSRPC / DCE/RPC interoperability / Applicability of FreeDCE for Wine (DCOM)

[Ge van Geldorp]

> From: Luke Kenneth Casson Leighton
> 
> what is my incentive to do that?

Ehm, perhaps because it's fun? There must be something with opensource that
makes all those volunteers want to contribute.

Were you asking for funding as many times on the samba mailing lists as on
the wine mailing lists? If so, I can begin to understand why they banned
you, it gets _really_ annoying.

Ge van Geldorp.

RE: [LOSTWAGES] Add janitorial task to avoid using strncpy

[Ge van Geldorp]

> From: Joris Huizer
> 
> Mike McCormack wrote:
> > ChangeLog:
> > * Add janitorial task to avoid using strncpy
> 
> Couldn't we just make a sane implementation of strncpy, not 
> adding more '\0' characters than necessary and making sure
> the last character is an '\0'?

That sane implementation already exists as lstrcpynA/lstrcpynW in kernel32.

Ge van Geldorp.

RE: advpack: add two missing stubs

[Ge van Geldorp]

> From: Dmitry Timoshkov
> 
> "Steven Edwards" <[EMAIL PROTECTED]> wrote:
> > Its nice to note where the code came from. We always try to 
> > do it in ReactOS so please credit Jim for his advpack function.
> 
> It's sad to say that, but unfortunately that's not the case 
> on the ReactOS side sometimes, especially for Wine code 
> borrowed at the early stage.

We (= ReactOS) really do try hard to credit whoever we need to give credit
to. For example, the installer lists the projects we borrowed code from
(with Wine at the top of the list). That doesn't free us from the obligation
to give credit in the source files ofcourse. If you have examples of where
we don't do that I'd be happy to add them.

Gé van Geldorp.

RE: Stubs in PE build

[Ge van Geldorp]

> From: Alexandre Julliard
>
> > Seems impossible for functions with unknown calling conventions and 
> > number of parameters.
> 
> I don't think that's a common case, most of these have been 
> identified by now. But anyway, if you have an app calling the 
> function you should be able to determine the number of 
> parameters; and if you don't then you shouldn't need the stub 
> at all. Do you have a specific case where this is a problem?

I was mixing native shell32.dll with Wine-derived shlwapi.dll. Native
shell32.dll imports those stubs from shlwapi.dll but then doesn't call them
in our usage scenarios.
I'm just a little bit surprised that stubs are ok for elf builds but not for
PE builds. It's not a big issue, I'm going to keep the code in ReactOS CVS
anyway .

Ge van Geldorp.

RE: Stubs in PE build

[Ge van Geldorp]

> From: [EMAIL PROTECTED]
> 
> I don't think we want to add a special mode just for stubs; 
> they should really be replaced by proper functions (even if 
> the functions are stubs themselves, at least they can print 
> the parameters and try to return something meaningful instead 
> of killing the app). You can view the lack of stubs support 
> on PE as an encouragement to help us remove them from the 
> spec files ;-)

Seems impossible for functions with unknown calling conventions and number
of parameters.

Ge van Geldorp.

RE: Treeview +/-

[Ge van Geldorp]

> From: Jon Griffiths
> 
> >Maybe the height/width at which to switch to the Rectangle drawing 
> >algorithm needs a bit of tuning?
> 
> Ge, Can you confirm this fixes the size for you? It works for 
> me if I change my system font to a larger one...

Yes, this works for me. Thanks.

Ge van Geldorp.

Stubs in PE build

[Ge van Geldorp]

Stubs declared in .spec files were not generated or exported for PE (MinGW)
builds. The patch below fixes that. It adds a new mode to winebuild, --pedll,
which generates the stubs.
Since it's a global change I'm sending it here for review first.

Ge van Geldorp.

Index: Make.rules.in
===
RCS file: /home/wine/wine/Make.rules.in,v
retrieving revision 1.179
diff -u -r1.179 Make.rules.in
--- Make.rules.in   7 Oct 2004 03:12:44 -   1.179
+++ Make.rules.in   24 Nov 2004 20:49:51 -
@@ -104,7 +104,7 @@
 api_manext  = 3w
 conf_manext = 5
 CLEAN_FILES = *.o *.a *.so *.ln *.$(LIBEXT) \\\#*\\\# *~ *% .\\\#* *.bak 
*.orig *.rej \
-  *.flc *.spec.c *.spec.def *.dbg.c *.tab.c *.tab.h @[EMAIL 
PROTECTED] core
+  *.flc *.spec.c *.spec.def *.dbg.c *.pedll.c *.tab.c *.tab.h 
@[EMAIL PROTECTED] core
 
 OBJS = $(C_SRCS:.c=.o) $(EXTRA_OBJS)
 
@@ -113,7 +113,7 @@
 
 # Implicit rules
 
-.SUFFIXES: .mc .rc .mc.rc .res .res.o .spec .spec.c .spec.def .idl .h .ok .sfd 
.ttf
+.SUFFIXES: .mc .rc .mc.rc .res .res.o .spec .spec.c .spec.def .pedll.c .idl .h 
.ok .sfd .ttf
 
 .c.o:
$(CC) -c $(ALLCFLAGS) -o $@ $<
@@ -135,6 +135,9 @@
 
 .spec.spec.def:
$(WINEBUILD) -w $(DEFS) -o $@ --def $<
+
+.spec.pedll.c:
+   $(WINEBUILD) $(DEFS) -o $@ --pedll $<
 
 .idl.h:
$(WIDL) $(IDLFLAGS) -h -H $@ $<
Index: dlls/Makedll.rules.in
===
RCS file: /home/wine/wine/dlls/Makedll.rules.in,v
retrieving revision 1.69
diff -u -r1.69 Makedll.rules.in
--- dlls/Makedll.rules.in   19 Oct 2004 23:06:12 -  1.69
+++ dlls/Makedll.rules.in   24 Nov 2004 20:49:53 -
@@ -31,8 +31,8 @@
 
 # Rules for .dll files
 
-$(MODULE): $(RCOBJS) $(OBJS) $(MODULE).dbg.o $(SPEC_DEF) $(IMPORTLIBS) 
Makefile.in
-   $(DLLWRAP) -k --def $(SPEC_DEF) -o $@ $(RCOBJS) $(OBJS) $(MODULE).dbg.o 
-L$(DLLDIR) $(DELAYIMPORTS:%=-l%) $(IMPORTS:%=-l%) $(ALL_LIBS)
+$(MODULE): $(RCOBJS) $(OBJS) $(MODULE).dbg.o $(MODULE:%.dll=%).pedll.o 
$(SPEC_DEF) $(IMPORTLIBS) $(DLLDIR)/libntdll.$(IMPLIBEXT) Makefile.in
+   $(DLLWRAP) -k --def $(SPEC_DEF) -o $@ $(RCOBJS) $(OBJS) $(MODULE).dbg.o 
$(MODULE:%.dll=%).pedll.o -L$(DLLDIR) $(DELAYIMPORTS:%=-l%) $(IMPORTS:%=-l%) 
-lntdll $(ALL_LIBS)
 
 $(SPEC_DEF): $(WINEBUILD)
 
Index: tools/winebuild/build.h
===
RCS file: /home/wine/wine/tools/winebuild/build.h,v
retrieving revision 1.54
diff -u -r1.54 build.h
--- tools/winebuild/build.h 8 Oct 2004 21:11:18 -   1.54
+++ tools/winebuild/build.h 24 Nov 2004 20:50:34 -
@@ -185,6 +185,7 @@
 extern void BuildSpec32File( FILE *outfile, DLLSPEC *spec );
 extern void BuildDef32File( FILE *outfile, DLLSPEC *spec );
 extern void BuildDebugFile( FILE *outfile, const char *srcdir, char **argv );
+extern void BuildPedllFile( FILE *outfile, DLLSPEC *spec );
 
 extern int parse_spec_file( FILE *file, DLLSPEC *spec );
 extern int parse_def_file( FILE *file, DLLSPEC *spec );
Index: tools/winebuild/main.c
===
RCS file: /home/wine/wine/tools/winebuild/main.c,v
retrieving revision 1.57
diff -u -r1.57 main.c
--- tools/winebuild/main.c  19 Oct 2004 23:09:02 -  1.57
+++ tools/winebuild/main.c  24 Nov 2004 20:50:34 -
@@ -77,7 +77,8 @@
 MODE_DEF,
 MODE_DEBUG,
 MODE_RELAY16,
-MODE_RELAY32
+MODE_RELAY32,
+MODE_PEDLL
 };
 
 static enum exec_mode_values exec_mode = MODE_NONE;
@@ -164,7 +165,8 @@
 "   --exe=NAME   Build a .c file for the named executable\n"
 "   --debug [FILES]  Build a .c file with the debug channels 
declarations\n"
 "   --relay16Build the 16-bit relay assembly routines\n"
-"   --relay32Build the 32-bit relay assembly routines\n\n"
+"   --relay32Build the 32-bit relay assembly routines\n"
+"   --pedll  Build a .c file for PE dll\n\n"
 "The mode options are mutually exclusive; you must specify one and only 
one.\n\n";
 
 enum long_options_values
@@ -178,7 +180,8 @@
 LONG_OPT_RELAY16,
 LONG_OPT_RELAY32,
 LONG_OPT_SUBSYSTEM,
-LONG_OPT_VERSION
+LONG_OPT_VERSION,
+LONG_OPT_PEDLL
 };
 
 static const char short_options[] = "C:D:F:H:I:K:L:M:N:d:e:f:hi:kl:m:o:r:w";
@@ -195,6 +198,7 @@
 { "relay32",  0, 0, LONG_OPT_RELAY32 },
 { "subsystem",1, 0, LONG_OPT_SUBSYSTEM },
 { "version",  0, 0, LONG_OPT_VERSION },
+{ "pedll",1, 0, LONG_OPT_PEDLL },
 /* aliases for short options */
 { "source-dir",1, 0, 'C' },
 { "delay-lib", 1, 0, 'd' },
@@ -356,6 +360,11 @@
 case LONG_OPT_VERSION:
 

Ekush

[Ge van Geldorp]

> From: Ge van Geldorp
>
> Ekush (http://www.ekush.com) published some binaries. 
> Surprise, surprise, it looks like ReactOS very much (Check 
> e.g. the radio buttons on the "Accept License" screen during 
> 2nd stage setup).
> Some string searching in their binaries reveals:
> 
> $ strings -a ver.dll | grep ReactOS
> OS2 Images not supported under ReactOS at this time.

Shortly after I sent this email to ros-dev yesterday the Ekush website went
down. Today they are back, with a slightly changed set of binaries. The
strings I reported yesterday are gone now (well, except for the
CcRosInitializeFileCache and CcRosReleaseFileCache exports from NTOSKRNL,
maybe changing the name of an exported symbol was too difficult???). There's
still plenty of other evidence of the ReactOS heritage though. I guess since
I only reported about ReactOS they forgot to remove the references to Wine
(except ofcourse the copyright notices, those are gone).

This pisses me off bigtime. I can only explain it as a deliberate attempt to
use the work of Qemu, FreeType, Wine and ReactOS without giving credit.

Gé van Geldorp.

RE: Treeview +/-

[Ge van Geldorp]

> From: Jon Griffiths
> 
> Hi,
> 
> Fixing my own FIXME.
> 
> Cheers,
> Jon
> 
>+wine/dlls/comctl32/treeview.c
> Draw +/- correctly for large icon sizes

(patch applied here:
http://www.winehq.org/hypermail/wine-cvs/2004/10/0115.html)

I can see that this would be an improvement for large icon sizes, but I
don't think it's good for small icon sizes. See attached screenshot of
regedit. Maybe the height/width at which to switch to the Rectangle drawing
algorithm needs a bit of tuning?

Ge van Geldorp.
<>

RE: dlls/setupapi/parser.c

[Ge van Geldorp]

> From: Alexandre Julliard
> 
> Ge van Geldorp <[EMAIL PROTECTED]> writes:
> 
> > Changelog:
> >   Ge van Geldorp <[EMAIL PROTECTED]>
> >   - GetFullPathName returns length without nul byte
> 
> If it does that's a bug, it's supposed to count the 
> terminating null. Or do you have evidence that it doesn't do 
> that on Windows?

No, I misread the MSDN documentation. Return value is length without nul
char if the buffer is large enough but length including nul char if the
buffer is not large enough. I'd like to have a word with the designer of
that API

Ge van Geldorp.

RE: wctype

[Ge van Geldorp]

> From: Dmitry Timoshkov
>
> "Ge van Geldorp" <[EMAIL PROTECTED]> wrote:
>
> > I compared the results of CT_CTYPE1 GetStringTypeW()
> > between Windows 2000 and Wine and found a lot of differences
>
> That's a known incompatibility. Another one is sort weight
> tables. We can do nothing to fix it, unicode.org and
> Microsoft have very different unicode tables.

Ok, I remember a discussion about sort weight tables some time ago.

> Do you have an app which depends on GetStringTypeW return values?

No, I was just porting some Wine routines to ReactOS and was a little bit
surprised when I tested the result and found the difference with Windows. I
guess we'll have to live with it just like you guys do.

Ge van Geldorp.

wctype

[Ge van Geldorp]

I compared the results of CT_CTYPE1 GetStringTypeW() between Windows 2000
and Wine and found a lot of differences (about 12000 of them). Nothing
really spectacular, e.g. codepoint 0x00b5 (MICRO SIGN) is classified
C1_PUNCT in Win2k and (C1_ALPHA | C1_LOWER) in Wine. I noticed that the Wine
table is generated from information on ftp.unicode.org, so I'm wondering if
the table generation is correct or if Microsoft made some strange choices?

Gé van Geldorp.

Moving some 16-bit code

[Ge van Geldorp]

dlls/shell32/control.c contains some 16-bit code (function RunDLL_CallEntry16),
which I would like to separate out. I wonder if the attached patch is the
correct way to do that?
On Win2k export 122 is just a stub. Maybe it would be ok to turn our export 122
into an empty function too?

Ge van Geldorp.

Index: control.c
===
RCS file: /home/wine/wine/dlls/shell32/control.c,v
retrieving revision 1.20
diff -u -r1.20 control.c
--- control.c   12 Jul 2004 19:50:56 -  1.20
+++ control.c   22 Sep 2004 19:28:18 -
@@ -456,30 +456,6 @@
 return Control_FillCache_RunDLLW(hWnd, hModule, w, x);
 }
 
-
-/*
- * RunDLL_CallEntry16  [SHELL32.122]
- * the name is probably wrong
- */
-void WINAPI RunDLL_CallEntry16( DWORD proc, HWND hwnd, HINSTANCE inst,
-LPCSTR cmdline, INT cmdshow )
-{
-WORD args[5];
-SEGPTR cmdline_seg;
-
-TRACE( "proc %lx hwnd %p inst %p cmdline %s cmdshow %d\n",
-   proc, hwnd, inst, debugstr_a(cmdline), cmdshow );
-
-cmdline_seg = MapLS( cmdline );
-args[4] = HWND_16(hwnd);
-args[3] = MapHModuleLS(inst);
-args[2] = SELECTOROF(cmdline_seg);
-args[1] = OFFSETOF(cmdline_seg);
-args[0] = cmdshow;
-WOWCallback16Ex( proc, WCB16_PASCAL, sizeof(args), args, NULL );
-UnMapLS( cmdline_seg );
-}
-
 /*
  * CallCPLEntry16  [SHELL32.166]
  *
Index: shell.c
===
RCS file: /home/wine/wine/dlls/shell32/shell.c,v
retrieving revision 1.61
diff -u -r1.61 shell.c
--- shell.c 15 Jun 2004 18:27:50 -  1.61
+++ shell.c 22 Sep 2004 19:28:18 -
@@ -657,3 +657,27 @@
 
 return HINSTANCE_16(seiW.hInstApp);
 }
+
+
+/*
+ * RunDLL_CallEntry16  [SHELL32.122]
+ * the name is probably wrong
+ */
+void WINAPI RunDLL_CallEntry16( DWORD proc, HWND hwnd, HINSTANCE inst,
+LPCSTR cmdline, INT cmdshow )
+{
+WORD args[5];
+SEGPTR cmdline_seg;
+
+TRACE( "proc %lx hwnd %p inst %p cmdline %s cmdshow %d\n",
+   proc, hwnd, inst, debugstr_a(cmdline), cmdshow );
+
+cmdline_seg = MapLS( cmdline );
+args[4] = HWND_16(hwnd);
+args[3] = MapHModuleLS(inst);
+args[2] = SELECTOROF(cmdline_seg);
+args[1] = OFFSETOF(cmdline_seg);
+args[0] = cmdshow;
+WOWCallback16Ex( proc, WCB16_PASCAL, sizeof(args), args, NULL );
+UnMapLS( cmdline_seg );
+}
Index: shell32.spec
===
RCS file: /home/wine/wine/dlls/shell32/shell32.spec,v
retrieving revision 1.87
diff -u -r1.87 shell32.spec
--- shell32.spec6 Sep 2004 20:24:10 -   1.87
+++ shell32.spec22 Sep 2004 19:28:19 -
@@ -115,7 +115,6 @@
  119 stdcall IsLFNDrive(ptr) IsLFNDriveAW
  120 stdcall FileMenu_AbortInitMenu ()
  121 stdcall SHFlushClipboard ()
- 122 stdcall -noname RunDLL_CallEntry16(long long long str long) #name wrong?
  123 stdcall SHFreeUnusedLibraries ()
  124 stdcall FileMenu_AppendFilesForPidl(long ptr long)
  125 stdcall FileMenu_AddFilesForPidl(long long long ptr long long ptr)
Index: shell.spec
===
RCS file: /home/wine/wine/dlls/shell32/shell.spec,v
retrieving revision 1.10
diff -u -r1.10 shell.spec
--- shell.spec  26 Sep 2003 04:32:20 -  1.10
+++ shell.spec  22 Sep 2004 19:28:19 -
@@ -29,6 +29,8 @@
 102 pascal -ret16 RegisterShellHook(word word) RegisterShellHook16
 103 pascal   ShellHookProc(word word long) ShellHookProc16
 
+122 pascal RunDLL_CallEntry16(long long long str long) #name wrong?
+
 157 stub RESTARTDIALOG
 #  166 PICKICONDLG
 

RE: RICHEDIT: use buffers rather than linked lists for input and out buffers

[Ge van Geldorp]

> From: Mike McCormack
> 
> This patch writes the output of the RTF parser into the edit 
> control in 
> 1024 byte chunks, and reads the input via a char[1024] buffer, rather 
> than a linked list of single characters.

Not that it should matter very much, but the MS control always seems to
read the input in 4096 byte chunks. Maybe you could enlarge InputBuffer
from 0x400 to 0x1000?

Ge van Geldorp.

RE: usb support in ROS

[Ge van Geldorp]

> From: Tom
> 
> Steven is USB by chance working in ROS? 
> http://cvs.reactos.com/cgi-bin/cvsweb.cgi/reactos/w32api/include/ddk/

No, not yet. Some work is being done on USB but it will take a while
before support is finished.

Ge van Geldorp.

Pending patches

[Ge van Geldorp]

Last week I submitted the following patches:

http://www.winehq.org/hypermail/wine-patches/2004/06/0227.html
(dlls/commdlg "old style" file dialogs - take 2)

and

http://www.winehq.org/hypermail/wine-patches/2004/06/0276.html
(dlls/shell32 RestartDialog and RestartDialogEx)

Is there any particular reason they haven't been accepted yet?

Gé van Geldorp.