> This is all pretty nifty :-) I notice, however, that the patch isn't quite
> perfect yet, and I'm not talking about code style :-)
>
> [snip]
> >+typedef BOOL (__stdcall *CrashGetLogFile) (void * crashstate);
>
> ^-- Good, but would be even better as "WINAPI" for those obscure systems
> that nobody ever uses any more that don't know what an __stdcall is.
You might remember that in the beginning, I had WINAPI there. I only changed
that to __stdcall in the last patch. Why? Because someone insisted that
there were still hungarian notations in the code and bad names. Since it
wasn't mentioned *what* exactly didn't match the standard, I was lost and
just changed *everything* I would even *remotely* consider MS style in
naming. And 'WINAPI' defines to __stdcall...
Sure, I could have changed one variable name, send the patch again, get the
same comment again, change another variable name, send patch again, get the
same comment again, change yet another variable name, send the patch
again,...
You see, trial-and-error with changing styling things is boring.
> ^-- Not quite as good. These functions are all from the same DLL as the
> first function, no? They probably should also have WINAPIs. I mean, the
> code works without them, but only because these typedefs are defaulting to
> __stdcall, since it is the default calling convention's default. Suppose
> someone decided to, for no reason discernible to us now, add /Gd onto the
> compile command-line for the subversion command-line tool. Then, the
> compiler would, as far as I know, assume that these function pointers all
> pointed at __cdecl functions, and very odd things would happen. The crash
> reporter would crash, resulting in mass confusion & hysteria!
Same as above. I just replaced the define 'WINAPI'.
> >+ FreeLibrary(crashDll);
> >+ crashDll = 0;
>
> ^-- What happens if this code runs twice by accident and you end up
> passing
> NULL into FreeLibrary? I'm not implying anything; I honestly don't know.
> For all I know, it'll just quietly return an error, but it might also
> segfault. I don't think the function is intended to be used on NULL
> pointers...
It will not segfault but return an error (invalid handle). If it bothers
you, you can move that line two lines up.
Stefan
--
GMX DSL = Maximale Leistung zum minimalen Preis!
2000 MB nur 2,99, Flatrate ab 4,99 Euro/Monat: http://www.gmx.net/de/go/dsl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jul 27 08:12:59 2005