[Mono-dev] Re: [PATCH] ShellExecuteEx ProcessId support forWindows 2000 and Windows XP RTM

[Kornél Pál]

Hi,

I'm only unsure about the configure part when specifying library (DLL) 
dependencies. I know that the solution is the link order but I don't know 
how to solve it. The submitted patch is correct but I'm not sure whether it 
is the best solution. I'm not going to submit another patch as I don't know 
other solution. If you have a better one please let me know.

I consider the actual code patch to be complete, stable and safe so I'm not 
going to modify it either.

I'm confident regarding modifications made to process.c. I'm sure that 
modifications made to configure.in are correct but there may be some other 
solution as well.

Checking for GetProcessId in header files is a bad idea as Windows (Platform 
SDK) header files are universal so if you define the appropriate WINVER 
(and/or _WIN32_WINNT, _WIN32_WINDOWS, _WIN32_IE) version you can link to the 
API (including GetProcessId) of that version regardless of the Windows 
version in use. Unlike the header files of Linux header files of Windows are 
not describing the actual operating system version.

But anyway this new code is not using GetProcessId on Windows and is using 
io-layer's implementation everywhere else so there is no need to check for 
GetProcessId.

Kornél

----- Original Message ----- 
From: "Dick Porter" <dick at ximian.com>
To: <mono-devel-list at lists.ximian.com>
Sent: Tuesday, April 04, 2006 2:18 PM
Subject: Re: [Mono-dev] Re: [PATCH] ShellExecuteEx ProcessId support 
forWindows 2000 and Windows XP RTM


On Tue, 2006-04-04 at 12:01 +0200, Kornél Pál wrote:
> Could someone please review this patch.

I was on holiday, and you didn't seem too confident about this patch
anyway; but here goes:

The GetProcessId check should remain in configure.in, and
HAVE_GETPROCESSID be used by the code to check if its available or not.
Remember, check for features not platforms.

Check that interdependencies between msvcrt.dll and ntdll.dll can be
_reliably_ solved by the link order.

Then I'll review an updated patch.

Thanks,

- Dick