Malcolm,
thanks for the review!
I've fixed most of your remarks in r23278, r23292-4 and r23330. Some
remarks follow:
Malcolm Rowe wrote:
>> + write_to_file(log_file,
>> + "eax=%08x ebx=%08x ecx=%08x edx=%08x esi=%08x edi=%08x\n",
>> + context->Eax, context->Ebx, context->Ecx,
>> + context->Edx, context->Esi, context->Edi);
>
> What happens with x64 or ia64 executables, incidentally?
Apparently the code doesn't build on x64. I'd rather leave this to fix
by someone with a 64-bit OS available to test. r23330 adds an #error
directive which stops the build on non-x86 platforms.
> Are you intending for this code to work when compiled with UNICODE
> defined? If yes, you just called CreateFileW with an ANSI filename. If
> no, why do you call GetTempPathA explicitly? (There are also other
> instances of this elsewhere, references to TCHAR etc).
Not really no, I fixed those instances which I found, there were only
two, did you see more?
>
> Finally, is it okay to assume that all command-line programs will be
> happy for us to add in our own exception handler? Seems a bit rude for
> a library to just assume that, though I doubt there's any real harm.
You mean applications that wrap svn.exe? Since the current behavior is
to hang the process with a dialog I don't presume it matters much.
Further I assume only the svn* executables to invoke svn_cmdline_init,
is that correct?
There's also the SVN_USE_WIN32_CRASHHANDLER define to disable building
the crash handler altogether.
Lieven
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 2 20:54:31 2007