Re: Simple arguments check

[Alexandre Julliard]

Mike Hearn  writes:
> Index: dlls/shell32/pidl.c
>
> RCS file: /home/wine/wine/dlls/shell32/pidl.c,v
> retrieving revision 1.94
> diff -u -r1.94 pidl.c
> --- dlls/shell32/pidl.c 4 Nov 2003 04:21:01 -0000       1.94
> +++ dlls/shell32/pidl.c 25 Dec 2003 19:32:48 -0000
> @@ -1234,10 +1234,19 @@
>         LPSHELLFOLDER shellfolder;
>     TRACE_(shell)("(pidl=%p,%p)\n",pidl,pszPath);
> -
>   pdump(pidl);
>
> -
>   if (!pidl) return FALSE;
> -
>   if (!pidl) {
> -
>       ERR_(shell)("argument check failed: pidl == null\n");
> -
>       return FALSE;
> -
>   }
> -
> -
>   if (!pszPath) {
> -
>       ERR_(shell)("argument check failed: pszPath == null\n");
> -
>       return FALSE;
> -
>   }

The ERRs are useless. If Windows does a NULL check, then we should do
one too, without any error message; if Windows doesn't, then we
shouldn't check at all.

Simple arguments check

[Mike Hearn]

On Tue, 2003-12-30 at 22:29, Alexandre Julliard wrote:
> The ERRs are useless. If Windows does a NULL check, then we should do
> one too, without any error message; if Windows doesn't, then we
> shouldn't check at all.

Well, I tested with a 98 copy of shell32 and it crashes too, so I guess
the patch was wrong anyway. I can't seem to reproduce the original
problem I was trying to solve anyway, so oh well, mark it for the X
files.

I guess the reason we need to crash when windows does is for apps that
trap the fault and rely on it? Or is it just about being as correct as
possible?

Simple arguments check

[Dimitrie O. Paun]

On December 31, 2003 12:14 pm, Mike Hearn wrote:
> I guess the reason we need to crash when windows does is for apps that
> trap the fault and rely on it? Or is it just about being as correct as
> possible?

Maybe because crashing is the right thing to do in many cases, but it
looks like MS really likes defensive programming and does all sort of
checks it shouldn't do (and catching all sort of exceptions it shouldn't
catch). However, this is no reason to the wrong thing and go down the
path of defensive programming where even MS didn't go.

In other words, many of these checks should be done only where MS does
them (since obviously apps will depend on them).

Simple arguments check

[Mike Hearn]

n Wed, 2003-12-31 at 17:36, Dimitrie O. Paun wrote:
> Maybe because crashing is the right thing to do in many cases,

Why? I'd have thought failing the API call would be more sensible, or
doing what GTK does and printing assertion failures. But hey. Crashing
is what Win32 does, so it's what we do. I know the answer really :)
> but it
> looks like MS really likes defensive programming and does all sort of
> checks it shouldn't do (and catching all sort of exceptions it shouldn't
> catch).

Some APIs even correct mistakes you make for you!

Simple arguments check

[Dimitrie O. Paun]

On December 31, 2003 03:29 pm, Mike Hearn wrote:
> > Maybe because crashing is the right thing to do in many cases,
>
> Why? I'd have thought failing the API call would be more sensible, or

Because it's bad policy: it's better to fail early and as close as
possible to the error point, rather then let the program limp along.

In general, MS made a bad policy call by trying to help badly written
apps to crawl along. This results in the long term in a lot of hard
to track bugs, slower/bloated code, etc. Remember that the road to
hell is paved with good intentions...