You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Lieven Govaerts <lg...@mobsol.be> on 2007/01/03 16:11:41 UTC

[PATCH] issue 1628: use a custom crashhandler on Windows

Attached is a patch that implements a crash handler (Windows specific)
for the command line apps. While I learned a lot from Stefan Küng's
patch posted here some time ago, it's basically a new implementation.
Review of my choices (see below) and the patch itself are greatly
appreciated.

Crash handler requirements:
- provide as much relevant information as possible about fatal exceptions
- allow non-Windows developers to be able to analyze the problem
- no need to install server-side tools to gather crash reports
- work in all Subversion command-line applications

Some choices I've made while working on this patch:
- works on Windows only
- no GUI
- don't put the crash handler code in a DLL
- only load the dbghelp DLL when needed
- inform the user about a crash, but let her decide to report the
information to us.
- only support dbghelp.dll >= 6.6.7.5, which means we'll have to include
this version in the win32 svn package.
- don't depend on apr
- don't build a data structure containing all crash info, write to the
logfile directly
- don't trigger the crash handler when the process runs in a debugger

What's included in this patch:
- report info about the process (cmd-line, version, platform)
- report info about the crash (exception, registers)
- include a stacktrace with function name, line nr, filename
- for each function parameter and local variable, try to print name and
value
- a message that informs the user about the crash and instructions for
sending the report to us

What's not included:
- write a message to the event log when svnserve is run as a service
- format values of user defined types

There's at least one thing that needs to be decided, and that's where to
send the crash report to. The minidump file can contain sensitive
information, so it's probably not a good idea to ask users to send it to
a public mailing list.

Lieven

[[[
Add a Windows-specific crash handler, capable of dumping line info and
stack
information in log and minidump format on disk.

* subversion/libsvn_subr/cmdline.c
  (svn_cmdline_init): on windows attach the crash handler.

* subversion/libsvn_subr/win32_crashrpt.c
* subversion/libsvn_subr/win32_crashrpt.h: New files, implement the crash
   handler.

* subversion/libsvn_subr/win32_crashrpt_dll.h: New file, definitions of the
   exported functions in the dbghelp, version and kernel32 DLL's.
]]]



Re: [PATCH] issue 1628: use a custom crashhandler on Windows

Posted by Ivan Zhakov <ch...@gmail.com>.
On 1/13/07, D.J. Heap <dj...@gmail.com> wrote:
> On 1/7/07, Ivan Zhakov <ch...@gmail.com> wrote:
> > On 1/3/07, Lieven Govaerts <lg...@mobsol.be> wrote:
> > >
> > > Attached is a patch that implements a crash handler (Windows specific)
> > > for the command line apps. While I learned a lot from Stefan Küng's
> > > patch posted here some time ago, it's basically a new implementation.
> > > Review of my choices (see below) and the patch itself are greatly
> > > appreciated.
>
>
> I've been testing this a bit and it looks very promising, thanks!
>
> I haven't reviewed it as thoroughly as Ivan, but I do agree with most
> of his comments except:
>
>
> >
> > many unbuffered writes to file. I suggest create file use run-time
> > function open(), then get Windows handle using _get_osfhandle() for
> > MiniDumpWrite.
>
>
> I don't think they are unbuffered -- I'm pretty sure the
> CreateFile/WriteFile API's do buffering unless you explicitly turn it
> off.
It's just term "buffered", because CRT have two differenet file APIs
open/read/write/close which some times called "unbuffered" and
fopen/fwrite/fread/fclose which is have user space buffer.

> And I'd say the less dependencies the better when doing a crash
> dump -- the CRT could be corrupt and not even able to write a file out
> (the whole process could be so corrupt that even CreateFile/WriteFile
> won't work, but it seems less likely than CRT corruption).
Good point, but anyway we use CRT for sprintf and other things. So in
this case it's doesn't matter to use fopen/fprintf to make code
simple.


-- 
Ivan Zhakov

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] issue 1628: use a custom crashhandler on Windows

Posted by "D.J. Heap" <dj...@gmail.com>.
On 1/7/07, Ivan Zhakov <ch...@gmail.com> wrote:
> On 1/3/07, Lieven Govaerts <lg...@mobsol.be> wrote:
> >
> > Attached is a patch that implements a crash handler (Windows specific)
> > for the command line apps. While I learned a lot from Stefan Küng's
> > patch posted here some time ago, it's basically a new implementation.
> > Review of my choices (see below) and the patch itself are greatly
> > appreciated.


I've been testing this a bit and it looks very promising, thanks!

I haven't reviewed it as thoroughly as Ivan, but I do agree with most
of his comments except:


>
> many unbuffered writes to file. I suggest create file use run-time
> function open(), then get Windows handle using _get_osfhandle() for
> MiniDumpWrite.


I don't think they are unbuffered -- I'm pretty sure the
CreateFile/WriteFile API's do buffering unless you explicitly turn it
off.  And I'd say the less dependencies the better when doing a crash
dump -- the CRT could be corrupt and not even able to write a file out
(the whole process could be so corrupt that even CreateFile/WriteFile
won't work, but it seems less likely than CRT corruption).

Also, although I see a good stack trace in the log file, I don't when
I open the minidump in VS2005 or WinDbg.  Are you able to get a stack
trace when inspecting the minidump with either debugger?

DJ

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] issue 1628: use a custom crashhandler on Windows

Posted by Lieven Govaerts <sv...@mobsol.be>.
Ivan Zhakov wrote:
>> >> +  va_start(argptr, format);
>> >> +  len = vsprintf(buf, format, argptr);
>> >> +  va_end( argptr );
>> > Unbounded sprintf. At least your should use vsnprintf or create FILE
>> > against handle and use fprintf.
>> Yep, you're right. I've changed it to use the CRT file handling
>> functions, this also solves the buffering problem (if any, I'm not so 
>> sure).
>
> Ok, but I see that you keep the function write_to_file which only
> forwards calls to fprintf. Why? You can just use fprintf for printing
> to file.
The original reason was to make it possible to change the log target, 
like an XML file or an internal data structure, but that changed later 
to directly writing to file, so there's indeed no real need anymore for 
this function.
> > The name check_dll_version is little bit confusing. I suggest name
>> > check_dbghelp_version().
>> The function supports any dll, but I've replaced it with
>> check_dbghelp_version now.
>> > GetFileVersionInfoSize, GetFileVersionInfo and VerQueryValue are
>> > supported on all versions of Windows so there is no need to call them
>> > using GetProcAddress.
>> The reason I load them here is because I don't want to load any extra
>> dll at process startup (or initialisation of the crash handler in this
>> case).
> I don't have strong objection against this code, but for me it's 
> overkill.
Ok, but do you agree that we shouldn't slow down start up of the svn 
processes to load a dll which is only used in case the process crashes? 
As far as I saw during my test sessions for this patch, this is the only 
way to achieve this.
>>
>> >> +      file = CreateFile(filename, GENERIC_WRITE, 0, NULL, 
>> CREATE_NEW,
>> >> +                        FILE_ATTRIBUTE_NORMAL, NULL);
>> > It's better to use access() for testing file presence.
>> access()?
> Yes, access is posix function and it's supported on Windows.
I couldn't find it in msdn, do you have an online reference available?

Do you have an opinion on the message to the end-user? I think we should 
the user tell explicitly not to send the minidump file to the mailing 
list, but I'd like to know what others think about it.

The code is on trunk now so feel free to change it.

Lieven

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] issue 1628: use a custom crashhandler on Windows

Posted by Ivan Zhakov <ch...@gmail.com>.
On 1/23/07, Lieven Govaerts <sv...@mobsol.be> wrote:
> Ivan, DJ,
>
> thanks for your review. I think your remarks (and some others) into
> account for the next version of the patch.
Lieven, I forgot about new version of your patch. Excuse me.

>
> Ivan Zhakov wrote:
> > On 1/3/07, Lieven Govaerts <lg...@mobsol.be> wrote:
> >>
> >> Attached is a patch that implements a crash handler (Windows specific)
> >> for the command line apps. While I learned a lot from Stefan Küng's
> >> patch posted here some time ago, it's basically a new implementation.
> >> Review of my choices (see below) and the patch itself are greatly
> >> appreciated.
> >>
> > Looks very nice! One thing that I didn't understand from this
> > description is where are minudump and log files get written?
> The log and minidump files are written in the temp folder. I prefer the
> temp folder over Subversion config folder, because these files don't
> have to be stored on the user's pc forever.
Ok, sounds reasonable.

>
> >> +  va_start(argptr, format);
> >> +  len = vsprintf(buf, format, argptr);
> >> +  va_end( argptr );
> > Unbounded sprintf. At least your should use vsnprintf or create FILE
> > against handle and use fprintf.
> Yep, you're right. I've changed it to use the CRT file handling
> functions, this also solves the buffering problem (if any, I'm not so sure).
Ok, but I see that you keep the function write_to_file which only
forwards calls to fprintf. Why? You can just use fprintf for printing
to file.

> >> +  size_t len = wcslen(str);
> >> +  char *utf8_str = malloc(sizeof(wchar_t) * len + 1);
> >> +  len = wcstombs(utf8_str, str, len);
> >> +  utf8_str[len] = '\0';
> > wcstombs can return -1 in some situation:
> > http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore98/HTML/_crt_wcstombs.asp
> >
> Ah ok, this situation is now handled.

[snip

> > many unbuffered writes to file. I suggest create file use run-time
> > function open(), then get Windows handle using _get_osfhandle() for
> > MiniDumpWrite.
> > And use standard stream API in this function.
> Writing to the log file is now buffered. I don't see how your suggestion
> works for MiniDumpWrite, you're basically getting the same handle as
> what you'd get using CreateFile no?
Sorry, I was thinking that you writing log to the same file as dump.
Just forget about my comment.

> > The name check_dll_version is little bit confusing. I suggest name
> > check_dbghelp_version().
> The function supports any dll, but I've replaced it with
> check_dbghelp_version now.
> > GetFileVersionInfoSize, GetFileVersionInfo and VerQueryValue are
> > supported on all versions of Windows so there is no need to call them
> > using GetProcAddress.
> The reason I load them here is because I don't want to load any extra
> dll at process startup (or initialisation of the crash handler in this
> case).
I don't have strong objection against this code, but for me it's overkill.

> >> I think it's better to check for NULL procedure address in one place.
> > I mean just:
> > if (!SymFromAddr_ ||
> >    !SymGetModuleBase_ ||
> >    ! ....)
> >  goto cleanup;
> Fixed.
> >> +  if (! GetTempPathA(MAX_PATH - 14, temp_dir))
> >> +    return;
> >> +
> >> +  for (i = 0;i < 3;i++)
> > Why try 3 times? What is the reason?
> If the file already exists (unlikely though as it includes the
> timestamp), we just try again with a new timestamp.
> >> +    {
> >> +      HANDLE file;
> >> +      time_t now;
> >> +      char time_str[64];
> >> +
> >> +      time(&now);
> >> +      strftime(time_str, 64, "%Y%m%d%H%M%S", localtime(&now));
> >> +      sprintf(filename, "%s%s%s.%s", temp_dir, prefix, time_str, ext);
> >> +
> >> +      file = CreateFile(filename, GENERIC_WRITE, 0, NULL, CREATE_NEW,
> >> +                        FILE_ATTRIBUTE_NORMAL, NULL);
> > It's better to use access() for testing file presence.
> access()?
Yes, access is posix function and it's supported on Windows.

> >> +  get_temp_filename(dmp_filename, "svn-crash-log", "dmp");
> >> +  get_temp_filename(log_filename, "svn-crash-log", "log");
> > Personally I prefer Application Data\Subversion for creating dump files.
> Why?
Just easier to find than temporary files. Anyway temporary directory
is ok for me.


-- 
Ivan Zhakov

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: [PATCH] issue 1628: use a custom crashhandler on Windows

Posted by Lieven Govaerts <sv...@mobsol.be>.
Ivan, DJ,

thanks for your review. I think your remarks (and some others) into
account for the next version of the patch.

Ivan Zhakov wrote:
> On 1/3/07, Lieven Govaerts <lg...@mobsol.be> wrote:
>>
>> Attached is a patch that implements a crash handler (Windows specific)
>> for the command line apps. While I learned a lot from Stefan Küng's
>> patch posted here some time ago, it's basically a new implementation.
>> Review of my choices (see below) and the patch itself are greatly
>> appreciated.
>>
> Looks very nice! One thing that I didn't understand from this
> description is where are minudump and log files get written?
The log and minidump files are written in the temp folder. I prefer the
temp folder over Subversion config folder, because these files don't
have to be stored on the user's pc forever.

>> +  va_start(argptr, format);
>> +  len = vsprintf(buf, format, argptr);
>> +  va_end( argptr );
> Unbounded sprintf. At least your should use vsnprintf or create FILE
> against handle and use fprintf.
Yep, you're right. I've changed it to use the CRT file handling
functions, this also solves the buffering problem (if any, I'm not so sure).
>> +  size_t len = wcslen(str);
>> +  char *utf8_str = malloc(sizeof(wchar_t) * len + 1);
>> +  len = wcstombs(utf8_str, str, len);
>> +  utf8_str[len] = '\0';
> wcstombs can return -1 in some situation:
> http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore98/HTML/_crt_wcstombs.asp
>
Ah ok, this situation is now handled.
>> +      default:
>> +        {
>> +          /* If not one of the "known" exceptions, try to get the
>> string from
>> +             NTDLL.DLL's message table. */
>> +          static char buf[512];
>> +
>> +          FormatMessage(FORMAT_MESSAGE_IGNORE_INSERTS |
>> +                        FORMAT_MESSAGE_FROM_HMODULE,
>> +                        GetModuleHandle("ntdll.dll"),
>> +                        exception, 0, buf, sizeof(buf), 0);
>> +
>> +          return buf;
> Hmm, returning pointer to static buffer which is dynamically modified.
> It's crazy and unsafe for me.
I've replaced it with "UNKNOWN_ERROR", that should be sufficient for
those rare cases.
> many unbuffered writes to file. I suggest create file use run-time
> function open(), then get Windows handle using _get_osfhandle() for
> MiniDumpWrite.
> And use standard stream API in this function.
Writing to the log file is now buffered. I don't see how your suggestion
works for MiniDumpWrite, you're basically getting the same handle as
what you'd get using CreateFile no?
> The name check_dll_version is little bit confusing. I suggest name
> check_dbghelp_version().
The function supports any dll, but I've replaced it with
check_dbghelp_version now.
> GetFileVersionInfoSize, GetFileVersionInfo and VerQueryValue are
> supported on all versions of Windows so there is no need to call them
> using GetProcAddress.
The reason I load them here is because I don't want to load any extra
dll at process startup (or initialisation of the crash handler in this
case).
>> I think it's better to check for NULL procedure address in one place.
> I mean just:
> if (!SymFromAddr_ ||
>    !SymGetModuleBase_ ||
>    ! ....)
>  goto cleanup;
Fixed.
>> +  if (! GetTempPathA(MAX_PATH - 14, temp_dir))
>> +    return;
>> +
>> +  for (i = 0;i < 3;i++)
> Why try 3 times? What is the reason?
If the file already exists (unlikely though as it includes the
timestamp), we just try again with a new timestamp.
>> +    {
>> +      HANDLE file;
>> +      time_t now;
>> +      char time_str[64];
>> +
>> +      time(&now);
>> +      strftime(time_str, 64, "%Y%m%d%H%M%S", localtime(&now));
>> +      sprintf(filename, "%s%s%s.%s", temp_dir, prefix, time_str, ext);
>> +
>> +      file = CreateFile(filename, GENERIC_WRITE, 0, NULL, CREATE_NEW,
>> +                        FILE_ATTRIBUTE_NORMAL, NULL);
> It's better to use access() for testing file presence.
access()?
>> +  get_temp_filename(dmp_filename, "svn-crash-log", "dmp");
>> +  get_temp_filename(log_filename, "svn-crash-log", "log");
> Personally I prefer Application Data\Subversion for creating dump files.
Why?
>> +
>> +  /* ... or if we can't create the log files ... */
>> +  if (*dmp_filename == '\0' || *log_filename == '\0')
>> +    return EXCEPTION_CONTINUE_SEARCH;
> get_temp_filename should return boolean to indicate is it right temp
> names generated instead of checking for empty name.
Fixed.

Attached patch some other fixes in the variable data presentation and
some other small improvements.

I've had some mixed results debugging the minidump file too, in fact,
because of the risk of the files including sensitive data and the fact
that it requires windows to parse them I preferred the make the log file
have a full stack trace including variable values. I'd actually rather
have the dump files are not mailed to a public mailing list, the log
files should be sufficient.

Lieven

Re: [PATCH] issue 1628: use a custom crashhandler on Windows

Posted by Ivan Zhakov <ch...@gmail.com>.
On 1/3/07, Lieven Govaerts <lg...@mobsol.be> wrote:
>
> Attached is a patch that implements a crash handler (Windows specific)
> for the command line apps. While I learned a lot from Stefan Küng's
> patch posted here some time ago, it's basically a new implementation.
> Review of my choices (see below) and the patch itself are greatly
> appreciated.
>
> Crash handler requirements:
> - provide as much relevant information as possible about fatal exceptions
> - allow non-Windows developers to be able to analyze the problem
> - no need to install server-side tools to gather crash reports
> - work in all Subversion command-line applications
>
> Some choices I've made while working on this patch:
> - works on Windows only
> - no GUI
> - don't put the crash handler code in a DLL
> - only load the dbghelp DLL when needed
> - inform the user about a crash, but let her decide to report the
> information to us.
> - only support dbghelp.dll >= 6.6.7.5, which means we'll have to include
> this version in the win32 svn package.
> - don't depend on apr
> - don't build a data structure containing all crash info, write to the
> logfile directly
> - don't trigger the crash handler when the process runs in a debugger
>
> What's included in this patch:
> - report info about the process (cmd-line, version, platform)
> - report info about the crash (exception, registers)
> - include a stacktrace with function name, line nr, filename
> - for each function parameter and local variable, try to print name and
> value
> - a message that informs the user about the crash and instructions for
> sending the report to us
>
> What's not included:
> - write a message to the event log when svnserve is run as a service
> - format values of user defined types
Looks very nice! One thing that I didn't understand from this
description is where are minudump and log files get written?

Look for my review inside.

>
> There's at least one thing that needs to be decided, and that's where to
> send the crash report to. The minidump file can contain sensitive
> information, so it's probably not a good idea to ask users to send it to
> a public mailing list.
>
> Lieven
>
> [[[
> Add a Windows-specific crash handler, capable of dumping line info and
> stack
> information in log and minidump format on disk.
>
> * subversion/libsvn_subr/cmdline.c
>   (svn_cmdline_init): on windows attach the crash handler.
>
> * subversion/libsvn_subr/win32_crashrpt.c
> * subversion/libsvn_subr/win32_crashrpt.h: New files, implement the crash
>    handler.
>
> * subversion/libsvn_subr/win32_crashrpt_dll.h: New file, definitions of the
>    exported functions in the dbghelp, version and kernel32 DLL's.
> ]]]
>
>
>
>
> Index: subversion/libsvn_subr/cmdline.c
> ===================================================================
> --- subversion/libsvn_subr/cmdline.c    (revision 22879)
> +++ subversion/libsvn_subr/cmdline.c    (working copy)
> @@ -44,6 +44,8 @@
>
>  #include "svn_private_config.h"
>
> +#include "win32_crashrpt.h"
> +
>  /* The stdin encoding. If null, it's the same as the native encoding. */
>  static const char *input_encoding = NULL;
>
> @@ -102,6 +104,10 @@
>      output_encoding = output_encoding_buffer;
>    }
>  #endif /* _MSC_VER < 1400 */
> +
> +  /* Attach (but don't load) the crash handler */
> +  SetUnhandledExceptionFilter(svn_unhandled_exception_filter);
> +
>  #endif /* WIN32 */
>
>    /* C programs default to the "C" locale. But because svn is supposed
> Index: subversion/libsvn_subr/win32_crashrpt.c
> ===================================================================
> --- subversion/libsvn_subr/win32_crashrpt.c     (revision 0)
> +++ subversion/libsvn_subr/win32_crashrpt.c     (revision 0)
> @@ -0,0 +1,813 @@
> +/*
> + * win32_crashrpt.c : provides information after a crash
> + *
> + * ====================================================================
> + * Copyright (c) 2007 CollabNet.  All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution.  The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals.  For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifdef WIN32
> +
> +/*** Includes. ***/
> +#include <windows.h>
> +#include <dbghelp.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include "svn_version.h"
> +
> +#include "win32_crashrpt.h"
> +#include "win32_crashrpt_dll.h"
> +
> +/*** Global variables ***/
> +HANDLE dbghelp_dll = INVALID_HANDLE_VALUE;
> +
> +/* email address where the crash reports should be sent too. */
> +#define crashreport_email "svnbreakage@subversion.tigris.org"
> +
> +/*** Code. ***/
> +
> +/* write string to file, allow printf style formatting */
> +static void
> +write_to_file(HANDLE file, const char *format, ...)
> +{
> +  char buf[1024];
> +  int len;
> +  DWORD written;
> +  va_list argptr;
> +
> +  va_start(argptr, format);
> +  len = vsprintf(buf, format, argptr);
> +  va_end( argptr );
Unbounded sprintf. At least your should use vsnprintf or create FILE
against handle and use fprintf.

> +
> +  WriteFile(file, buf, len, &written, NULL);
> +}
> +
> +/* Convert a wide-character string to utf-8. This function will create a buffer
> + * large enough to hold the result string, the caller should free this buffer.
> + */
> +static char *
> +convert_wbcs_to_utf8(const wchar_t *str)
> +{
> +  size_t len = wcslen(str);
> +  char *utf8_str = malloc(sizeof(wchar_t) * len + 1);
> +  len = wcstombs(utf8_str, str, len);
> +  utf8_str[len] = '\0';
wcstombs can return -1 in some situation:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore98/HTML/_crt_wcstombs.asp

> +
> +  return utf8_str;
> +}
> +
> +/* Convert the exception code to a string */
> +static const char *
> +exception_string(int exception)
> +{
> +  #define EXCEPTION(x) case EXCEPTION_##x: return (#x);
> +
> +  switch (exception)
> +    {
> +      EXCEPTION(ACCESS_VIOLATION)
> +      EXCEPTION(DATATYPE_MISALIGNMENT)
> +      EXCEPTION(BREAKPOINT)
> +      EXCEPTION(SINGLE_STEP)
> +      EXCEPTION(ARRAY_BOUNDS_EXCEEDED)
> +      EXCEPTION(FLT_DENORMAL_OPERAND)
> +      EXCEPTION(FLT_DIVIDE_BY_ZERO)
> +      EXCEPTION(FLT_INEXACT_RESULT)
> +      EXCEPTION(FLT_INVALID_OPERATION)
> +      EXCEPTION(FLT_OVERFLOW)
> +      EXCEPTION(FLT_STACK_CHECK)
> +      EXCEPTION(FLT_UNDERFLOW)
> +      EXCEPTION(INT_DIVIDE_BY_ZERO)
> +      EXCEPTION(INT_OVERFLOW)
> +      EXCEPTION(PRIV_INSTRUCTION)
> +      EXCEPTION(IN_PAGE_ERROR)
> +      EXCEPTION(ILLEGAL_INSTRUCTION)
> +      EXCEPTION(NONCONTINUABLE_EXCEPTION)
> +      EXCEPTION(STACK_OVERFLOW)
> +      EXCEPTION(INVALID_DISPOSITION)
> +      EXCEPTION(GUARD_PAGE)
> +      EXCEPTION(INVALID_HANDLE)
> +
> +      default:
> +        {
> +          /* If not one of the "known" exceptions, try to get the string from
> +             NTDLL.DLL's message table. */
> +          static char buf[512];
> +
> +          FormatMessage(FORMAT_MESSAGE_IGNORE_INSERTS |
> +                        FORMAT_MESSAGE_FROM_HMODULE,
> +                        GetModuleHandle("ntdll.dll"),
> +                        exception, 0, buf, sizeof(buf), 0);
> +
> +          return buf;
Hmm, returning pointer to static buffer which is dynamically modified.
It's crazy and unsafe for me.

> +        }
> +    }
> +}
> +
> +/* Write the minidump to file. The callback function will at the same time
> +   write the list of modules to the log file. */
> +static BOOL
> +write_minidump_file(const char *file, PEXCEPTION_POINTERS ptrs,
> +                    MINIDUMP_CALLBACK_ROUTINE module_callback,
> +                    void *data)
> +{
> +  /* open minidump file */
> +  HANDLE minidump_file = CreateFile(file, GENERIC_WRITE, 0, NULL,
> +                                    CREATE_ALWAYS,
> +                                    FILE_ATTRIBUTE_NORMAL,
> +                                    NULL);
> +
> +  if (minidump_file != INVALID_HANDLE_VALUE)
> +    {
> +      MINIDUMP_EXCEPTION_INFORMATION expt_info;
> +      MINIDUMP_CALLBACK_INFORMATION dump_cb_info;
> +
> +      expt_info.ThreadId = GetCurrentThreadId();
> +      expt_info.ExceptionPointers = ptrs;
> +      expt_info.ClientPointers = FALSE;
> +
> +      dump_cb_info.CallbackRoutine = module_callback;
> +      dump_cb_info.CallbackParam = data;
> +
> +      MiniDumpWriteDump_(GetCurrentProcess(),
> +                         GetCurrentProcessId(),
> +                         minidump_file,
> +                         MiniDumpNormal,
> +                         ptrs ? &expt_info : NULL,
> +                         NULL,
> +                         &dump_cb_info);
> +
> +      CloseHandle(minidump_file);
> +      return TRUE;
> +    }
> +
> +  return FALSE;
> +}
> +
> +/* Write module information to the log file */
> +static BOOL CALLBACK
> +write_module_info_callback(void *data,
> +                 CONST PMINIDUMP_CALLBACK_INPUT callback_input,
> +                 PMINIDUMP_CALLBACK_OUTPUT callback_output)
> +{
> +  if (data != NULL &&
> +      callback_input != NULL &&
> +      callback_input->CallbackType == ModuleCallback)
> +    {
> +      HANDLE log_file = (HANDLE)data;
> +      MINIDUMP_MODULE_CALLBACK module = callback_input->Module;
> +
> +      char *buf = convert_wbcs_to_utf8(module.FullPath);
> +      write_to_file(log_file, "0x%08x", module.BaseOfImage);
> +      write_to_file(log_file, "  %s", buf);
> +      free(buf);
> +
> +      write_to_file(log_file, " (%d.%d.%d.%d, %d bytes)\n",
> +                              HIWORD(module.VersionInfo.dwFileVersionMS),
> +                              LOWORD(module.VersionInfo.dwFileVersionMS),
> +                              HIWORD(module.VersionInfo.dwFileVersionLS),
> +                              LOWORD(module.VersionInfo.dwFileVersionLS),
> +                              module.SizeOfImage);
> +    }
> +
> +  return TRUE;
> +}
> +
> +/* Write details about the current process, platform and the exception */
> +static void
> +write_process_info(EXCEPTION_RECORD *exception, CONTEXT *context,
> +                   HANDLE log_file)
> +{
> +  OSVERSIONINFO oi;
> +  const char *cmd_line;
> +
> +  /* write the command line */
> +  cmd_line = GetCommandLine();
> +  write_to_file(log_file,
> +                "Cmd line: %.65s\n", cmd_line);
> +
> +  /* write the svn version number info. */
> +  write_to_file(log_file,
> +                "Version:  %s, compiled %s, %s\n",
> +                SVN_VERSION, __DATE__, __TIME__);
> +
> +  /* write information about the OS */
> +  oi.dwOSVersionInfoSize = sizeof(oi);
> +  GetVersionEx(&oi);
> +
> +  write_to_file(log_file,
> +                "Platform: Windows OS version %d.%d build %d %s\n\n",
> +                oi.dwMajorVersion, oi.dwMinorVersion, oi.dwBuildNumber,
> +                oi.szCSDVersion);
> +
> +  /* write the exception code */
> +  write_to_file(log_file,
> +               "Exception: %s\n\n",
> +               exception_string(exception->ExceptionCode));
> +
> +  /* write the register info. */
> +  write_to_file(log_file,
> +                "Registers:\n");
> +  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);
> +  write_to_file(log_file,
> +                "eip=%08x esp=%08x ebp=%08x efl=%08x\n",
> +                context->Eip, context->Esp,
> +                context->Ebp, context->EFlags);
> +  write_to_file(log_file,
> +                "cd=%04x  ss=%04x  ds=%04x  es=%04x  fs=%04x  gs=%04x\n",
> +                context->SegCs, context->SegSs, context->SegDs,
> +                context->SegEs, context->SegFs, context->SegGs);

many unbuffered writes to file. I suggest create file use run-time
function open(), then get Windows handle using _get_osfhandle() for
MiniDumpWrite.
And use standard stream API in this function.

> +}
> +
> +/* formats the value at address based on the specified basic type
> + * (char, int, long ...). */
> +static void
> +format_basic_type(char *buf, DWORD basic_type, DWORD64 length, void *address)
> +{
> +  switch(length)
> +    {
> +      case 1:
> +        sprintf(buf, "%x", *(unsigned char *)address);
> +        break;
> +      case 2:
> +        sprintf(buf, "%x", *(unsigned short *)address);
> +        break;
> +      case 4:
> +        switch(basic_type)
> +          {
> +            case 2:  /* btChar */
> +              {
> +                if (!IsBadStringPtr(*(PSTR*)address, 32))
> +                  sprintf(buf, "\"%.31s\"", *(unsigned long *)address);
> +                else
> +                  sprintf(buf, "%x", *(unsigned long *)address);
> +              }
> +            case 6:  /* btInt */
> +              sprintf(buf, "%d", *(int *)address);
> +              break;
> +            case 8:  /* btFloat */
> +              sprintf(buf, "%f", *(float *)address);
> +              break;
> +            default:
> +              sprintf(buf, "%x", *(unsigned long *)address);
> +              break;
> +          }
> +        break;
> +      case 8:
> +        if (basic_type == 8) /* btFloat */
> +          sprintf(buf, "%lf", *(double *)address);
> +        else
> +          sprintf(buf, "%I64X", *(unsigned __int64 *)address);
> +        break;
> +    }
> +}
> +
> +/* formats the value at address based on the type (pointer, user defined,
> + * basic type). */
> +static void
> +format_value(char *value_str, DWORD64 mod_base, DWORD type, void *value_addr)
> +{
> +  DWORD tag;
> +  int ptr = 0;
> +  HANDLE proc = GetCurrentProcess();
> +
> +  while (SymGetTypeInfo_(proc, mod_base, type, TI_GET_SYMTAG, &tag))
> +    {
> +      /* SymTagPointerType */
> +      if (tag == 14)
> +        {
> +          ptr++;
> +          SymGetTypeInfo_(proc, mod_base, type, TI_GET_TYPE, &type);
> +          continue;
> +        }
> +      break;
> +    }
> +
> +  switch(tag)
> +    {
> +      case 11: /* SymTagUDT */
> +        {
> +          WCHAR *type_name_wbcs;
> +          if (SymGetTypeInfo_(proc, mod_base, type, TI_GET_SYMNAME,
> +                              &type_name_wbcs))
> +            {
> +              char *type_name = convert_wbcs_to_utf8(type_name_wbcs);
> +              LocalFree(type_name_wbcs);
> +
> +              if (ptr == 0)
> +                sprintf(value_str, "(%s) 0x%08x",
> +                        type_name, (DWORD *)value_addr);
> +              else if (ptr == 1)
> +                sprintf(value_str, "(%s *) 0x%08x",
> +                        type_name, *(DWORD *)value_addr);
> +              else
> +                sprintf(value_str, "(%s **) 0x%08x",
> +                        type_name, (DWORD *)value_addr);
> +
> +              free(type_name);
> +            }
> +        }
> +        break;
> +      case 16: /* SymTagBaseType */
> +        {
> +          DWORD bt;
> +          ULONG64 length;
> +          SymGetTypeInfo_(proc, mod_base, type, TI_GET_LENGTH, &length);
> +
> +          /* print a char * as a string */
> +          if (ptr == 1 && length == 1)
> +            {
> +              sprintf(value_str, "0x%08x \"%s\"",
> +                      *(DWORD *)value_addr, (char *)value_addr);
> +              break;
> +            }
> +          if (ptr >= 1)
> +            {
> +              sprintf(value_str, "0x%08x", *(DWORD *)value_addr);
> +              break;
> +            }
> +          if (SymGetTypeInfo_(proc, mod_base, type, TI_GET_BASETYPE, &bt))
> +            {
> +              format_basic_type(value_str, bt, length, value_addr);
> +              break;
> +            }
> +        }
> +        break;
> +      default: printf("tag: %d", tag); break; /* TODO remove */
> +    }
> +}
> +
> +/* Internal structure used to pass some data to the enumerate symbols
> + * callback */
> +typedef struct {
> +  STACKFRAME *stack_frame;
> +  HANDLE log_file;
> +  int nr_of_frame;
> +  BOOL log_params;
> +} symbols_baton_t;
> +
> +/* write the details of one parameter or local variable to the log file */
> +static BOOL WINAPI
> +write_var_values(PSYMBOL_INFO sym_info, ULONG sym_size, void *baton)
> +{
> +  static int last_nr_of_frame = 0;
> +  DWORD_PTR var_data = 0;    /* Will point to the variable's data in memory */
> +  STACKFRAME *stack_frame = ((symbols_baton_t*)baton)->stack_frame;
> +  HANDLE log_file = ((symbols_baton_t*)baton)->log_file;
> +  int nr_of_frame = ((symbols_baton_t*)baton)->nr_of_frame;
> +  BOOL log_params = ((symbols_baton_t*)baton)->log_params;
> +  char value_str[256] = "";
> +
> +  /* get the variable's data */
> +  if (sym_info->Flags & SYMFLAG_REGREL)
> +    {
> +      var_data = stack_frame->AddrFrame.Offset;
> +      var_data += (DWORD_PTR)sym_info->Address;
> +    }
> +  else
> +    return FALSE;
> +
> +  if (log_params == TRUE && sym_info->Flags & SYMFLAG_PARAMETER)
> +    {
> +      if (last_nr_of_frame == nr_of_frame)
> +        write_to_file(log_file, ", ", 2);
> +      else
> +        last_nr_of_frame = nr_of_frame;
> +
> +      format_value((char *)value_str, sym_info->ModBase, sym_info->TypeIndex,
> +                   (void *)var_data);
> +      write_to_file(log_file, "%s=%s", sym_info->Name, value_str);
> +    }
> +  if (log_params == FALSE && sym_info->Flags & SYMFLAG_LOCAL)
> +    {
> +      format_value((char *)value_str, sym_info->ModBase, sym_info->TypeIndex,
> +                   (void *)var_data);
> +      write_to_file(log_file, "        %s = %s\n", sym_info->Name, value_str);
> +    }
> +
> +  return TRUE;
> +}
> +
> +/* write the details of one function to the log file */
> +static void
> +write_function_detail(STACKFRAME stack_frame, void *data)
> +{
> +  ULONG64 symbolBuffer[(sizeof(SYMBOL_INFO) +
> +    MAX_PATH*sizeof(TCHAR) +
> +    sizeof(ULONG64) - 1) /
> +    sizeof(ULONG64)];
> +  PSYMBOL_INFO pIHS = (PSYMBOL_INFO)symbolBuffer;
> +  DWORD64 func_disp=0 ;
> +
> +  IMAGEHLP_STACK_FRAME ih_stack_frame;
> +  IMAGEHLP_LINE ih_line;
> +       DWORD line_disp=0;
> +
> +  HANDLE proc = GetCurrentProcess();
> +  HANDLE log_file = (HANDLE)data;
> +
> +  symbols_baton_t ensym;
> +
> +  static int nr_of_frame = 0;
> +
> +  nr_of_frame++;
> +
> +  /* log the function name */
> +  pIHS->SizeOfStruct = sizeof(SYMBOL_INFO);
> +  pIHS->MaxNameLen = MAX_PATH;
> +  if (SymFromAddr_(proc, stack_frame.AddrPC.Offset, &func_disp, pIHS) == FALSE)
> +    return;
> +
> +  write_to_file(log_file,
> +                "#%d  0x%08x in %.200s (",
> +                nr_of_frame, stack_frame.AddrPC.Offset,  pIHS->Name);
> +
> +  /* restrict symbol enumeration to this frame only */
> +  ih_stack_frame.InstructionOffset = stack_frame.AddrPC.Offset;
> +  SymSetContext_(proc, &ih_stack_frame, 0);
> +
> +  ensym.log_file = log_file;
> +  ensym.stack_frame = &stack_frame;
> +  ensym.nr_of_frame = nr_of_frame;
> +
> +  /* log all function parameters */
> +  ensym.log_params = TRUE;
> +  SymEnumSymbols_(proc, 0, 0, write_var_values, &ensym);
> +
> +  write_to_file(log_file, ")", 1);
> +
> +  /* find the source line for this function. */
> +  ih_line.SizeOfStruct = sizeof(IMAGEHLP_LINE);
> +  if (SymGetLineFromAddr_(proc, stack_frame.AddrPC.Offset,
> +                          &line_disp, &ih_line) != 0)
> +    {
> +      write_to_file(log_file,
> +                    " at %s:%d\n", ih_line.FileName, ih_line.LineNumber);
> +    }
> +
> +  /* log all function local variables */
> +  ensym.log_params = FALSE;
> +  SymEnumSymbols_(proc, 0, 0, write_var_values, &ensym);
> +}
> +
> +/* walk over the stack and log all relevant information to the log file */
> +static void
> +write_stacktrace(CONTEXT *context, HANDLE log_file)
> +{
> +  HANDLE proc = GetCurrentProcess();
> +  STACKFRAME stack_frame;
> +  DWORD machine;
> +  CONTEXT ctx;
> +  int skip = 0, i = 0;
> +
> +  /* The thread information - if not supplied. */
> +       if (context == NULL)
> +    {
> +      /* If no context is supplied, skip 1 frame */
> +      skip = 1;
> +
> +               ctx.ContextFlags = CONTEXT_FULL ;
> +               if (GetThreadContext(GetCurrentThread(), &ctx))
> +                   context = &ctx;
> +         }
> +
> +       if (context == NULL)
> +    return;
> +
> +  /* Write the stack trace */
> +  ZeroMemory(&stack_frame, sizeof(STACKFRAME));
> +  stack_frame.AddrPC.Mode = AddrModeFlat ;
> +  stack_frame.AddrStack.Mode   = AddrModeFlat ;
> +  stack_frame.AddrFrame.Mode   = AddrModeFlat ;
> +#if defined (_M_IX86)
> +  machine = IMAGE_FILE_MACHINE_I386 ;
> +  stack_frame.AddrPC.Offset    = context->Eip;
> +  stack_frame.AddrStack.Offset = context->Esp;
> +  stack_frame.AddrFrame.Offset = context->Ebp;
> +#elif defined (_M_AMD64)
> +  machine = IMAGE_FILE_MACHINE_AMD64 ;
> +  stack_frame.AddrPC.Offset    = context->Rip;
> +  stack_frame.AddrStack.Offset = context->Rsp;
> +  stack_frame.AddrFrame.Offset = context->Rbp;
> +#endif
> +
> +  while (1)
> +    {
> +      if (! StackWalk_(machine, proc, GetCurrentThread(),
> +                      &stack_frame, context, NULL, SymFunctionTableAccess_,
> +                      SymGetModuleBase_, NULL))
> +        {
> +          break;
> +        }
> +
> +      if (i >= skip)
> +        {
> +          /* Try to include symbolic information.
> +             Also check that the address is not zero. Sometimes StackWalk
> +             returns TRUE with a frame of zero. */
> +          if (stack_frame.AddrPC.Offset != 0)
> +            {
> +              write_function_detail(stack_frame, (void *)log_file);
> +            }
> +        }
> +      i++;
> +    }
> +}
> +
> +/* Check if a debugger is attached to this process */
> +static BOOL
> +is_debugger_present()
> +{
> +  HANDLE kernel32_dll = LoadLibrary("kernel32.dll");
> +  BOOL result;
> +
> +  ISDEBUGGERPRESENT IsDebuggerPresent_ =
> +          (ISDEBUGGERPRESENT)GetProcAddress(kernel32_dll, "IsDebuggerPresent");
> +
> +  if (IsDebuggerPresent_ && IsDebuggerPresent_())
> +    result = TRUE;
> +  else
> +    result = FALSE;
> +
> +  FreeLibrary(kernel32_dll);
> +
> +  return result;
> +}
> +
> +/* Match the version of dbghelp.dll with the minimum expected version */
> +static BOOL
> +check_dll_version(const char *path, WORD exp_major, WORD exp_minor,
The name check_dll_version is little bit confusing. I suggest name
check_dbghelp_version().

> +                  WORD exp_build, WORD exp_qfe)
> +{
> +  HANDLE version_dll = LoadLibrary("version.dll");
> +  GETFILEVERSIONINFOSIZE GetFileVersionInfoSize_ =
> +         (GETFILEVERSIONINFOSIZE)GetProcAddress(version_dll,
> +                                                "GetFileVersionInfoSizeA");
> +  GETFILEVERSIONINFO GetFileVersionInfo_ =
> +         (GETFILEVERSIONINFO)GetProcAddress(version_dll,
> +                                            "GetFileVersionInfoA");
> +  VERQUERYVALUE VerQueryValue_ =
> +         (VERQUERYVALUE)GetProcAddress(version_dll, "VerQueryValueA");
> +
> +  DWORD version     = 0,
> +        exp_version = MAKELONG(MAKEWORD(exp_qfe, exp_build),
> +                               MAKEWORD(exp_minor, exp_major));
GetFileVersionInfoSize, GetFileVersionInfo and VerQueryValue are
supported on all versions of Windows so there is no need to call them
using GetProcAddress.

> +  DWORD h = 0;
> +  DWORD resource_size = GetFileVersionInfoSize_(path, &h);
> +
> +  if(resource_size)
> +    {
> +      void *resource_data = malloc(resource_size);
> +      if(GetFileVersionInfo_(path, h, resource_size, resource_data) != FALSE)
> +        {
> +          void *buf = NULL;
> +          UINT len;
> +          if (VerQueryValue_(resource_data, "\\", &buf, &len))
> +            {
> +              VS_FIXEDFILEINFO *info = (VS_FIXEDFILEINFO*)buf;
> +              version = MAKELONG(MAKEWORD(LOWORD(info->dwFileVersionLS),
> +                                          HIWORD(info->dwFileVersionLS)),
> +                                 MAKEWORD(LOWORD(info->dwFileVersionMS),
> +                                          HIWORD(info->dwFileVersionMS)));
> +            }
> +        }
> +      free(resource_data);
> +    }
> +
> +   FreeLibrary(version_dll);
> +
> +   if (version >= exp_version)
> +     return TRUE;
> +
> +   return FALSE;
> +}
> +
> +/* Load the dbghelp.dll file, try to find a version that matches our
> +   requirements. */
> +static BOOL
> +load_dbghelp_dll()
> +{
> +  /* check version of the dll, should be at least 6.6.7.5 */
> +  if (check_dll_version("dbghelp.dll", 6, 6, 7, 5) == FALSE)
if (!check_dll_version())
   return FALSE;

> +    return FALSE;
> +
> +  dbghelp_dll = LoadLibrary("dbghelp.dll");
> +  if (dbghelp_dll != INVALID_HANDLE_VALUE)
> +    {
> +      DWORD opts;
> +
> +      /* load the functions */
> +      MiniDumpWriteDump_ =
> +           (MINIDUMPWRITEDUMP)GetProcAddress(dbghelp_dll, "MiniDumpWriteDump");
> +      if (! MiniDumpWriteDump_)
> +        goto cleanup;
> +
> +      SymInitialize_ =
> +           (SYMINITIALIZE)GetProcAddress(dbghelp_dll, "SymInitialize");
> +      if (! SymInitialize_)
> +        goto cleanup;
> +
> +      SymSetOptions_ =
> +           (SYMSETOPTIONS)GetProcAddress(dbghelp_dll, "SymSetOptions");
> +      if (! SymSetOptions_)
> +        goto cleanup;
> +
> +      SymGetOptions_ =
> +           (SYMGETOPTIONS)GetProcAddress(dbghelp_dll, "SymGetOptions");
> +      if (! SymGetOptions_)
> +        goto cleanup;
> +
> +      SymCleanup_ =
> +           (SYMCLEANUP)GetProcAddress(dbghelp_dll, "SymCleanup");
> +      if (! SymCleanup_)
> +        goto cleanup;
> +
> +      SymGetTypeInfo_ =
> +           (SYMGETTYPEINFO)GetProcAddress(dbghelp_dll, "SymGetTypeInfo");
> +      if (! SymGetTypeInfo_)
> +        goto cleanup;
> +
> +      SymGetLineFromAddr_ =
> +           (SYMGETLINEFROMADDR)GetProcAddress(dbghelp_dll,
> +                                              "SymGetLineFromAddr");
> +      if (! SymGetLineFromAddr_)
> +        goto cleanup;
> +
> +      SymEnumSymbols_ =
> +           (SYMENUMSYMBOLS)GetProcAddress(dbghelp_dll, "SymEnumSymbols");
> +      if (! SymEnumSymbols_)
> +        goto cleanup;
> +
> +      SymSetContext_ =
> +           (SYMSETCONTEXT)GetProcAddress(dbghelp_dll, "SymSetContext");
> +      if (! SymSetContext_)
> +        goto cleanup;
> +
> +      SymFromAddr_ = (SYMFROMADDR)GetProcAddress(dbghelp_dll, "SymFromAddr");
> +      if (! SymFromAddr_)
> +        goto cleanup;
> +
> +      StackWalk_ = (STACKWALK)GetProcAddress(dbghelp_dll, "StackWalk");
> +      if (! StackWalk_)
> +        goto cleanup;
> +
> +      SymFunctionTableAccess_ =
> +           (SYMFUNCTIONTABLEACCESS)GetProcAddress(dbghelp_dll,
> +                                                  "SymFunctionTableAccess");
> +      if (! SymFunctionTableAccess_)
> +        goto cleanup;
> +
> +      SymGetModuleBase_ =
> +           (SYMGETMODULEBASE)GetProcAddress(dbghelp_dll, "SymGetModuleBase");
> +      if (! SymGetModuleBase_)
> +        goto cleanup;
I think it's better to check for NULL procedure address in one place.
I mean just:
if (!SymFromAddr_ ||
    !SymGetModuleBase_ ||
    ! ....)
  goto cleanup;

> +
> +      /* initialize the symbol loading code */
> +      opts = SymGetOptions_();
> +
> +      /* Set the 'load lines' option to retrieve line number information;
> +         set the Deferred Loads option to map the debug info in memory only
> +         when needed. */
> +      SymSetOptions_(opts | SYMOPT_LOAD_LINES | SYMOPT_DEFERRED_LOADS);
> +
> +      /* Initialize the debughlp DLL with the default path and automatic
> +         module enumeration (and loading of symbol tables) for this process.
> +       */
> +      SymInitialize_(GetCurrentProcess(), NULL, TRUE);
> +
> +      return TRUE;
> +    }
> +
> +cleanup:
> +  if (dbghelp_dll)
> +    FreeLibrary(dbghelp_dll);
> +
> +  return FALSE;
> +}
> +
> +/* Cleanup the dbghelp.dll library */
> +static void
> +cleanup_debughlp()
> +{
> +  SymCleanup_(GetCurrentProcess());
> +
> +  FreeLibrary(dbghelp_dll);
> +}
> +
> +/* Create a filename based on a prefix, the timestamp and an extension.
> +   check if the filename was already taken, retry 3 times. */
> +void
> +get_temp_filename(char *filename, const char *prefix, const char *ext)
> +{
> +  char temp_dir[MAX_PATH - 14];
> +  int i;
> +
> +  if (! GetTempPathA(MAX_PATH - 14, temp_dir))
> +    return;
> +
> +  for (i = 0;i < 3;i++)
Why try 3 times? What is the reason?

> +    {
> +      HANDLE file;
> +      time_t now;
> +      char time_str[64];
> +
> +      time(&now);
> +      strftime(time_str, 64, "%Y%m%d%H%M%S", localtime(&now));
> +      sprintf(filename, "%s%s%s.%s", temp_dir, prefix, time_str, ext);
> +
> +      file = CreateFile(filename, GENERIC_WRITE, 0, NULL, CREATE_NEW,
> +                        FILE_ATTRIBUTE_NORMAL, NULL);
It's better to use access() for testing file presence.

> +      if (file != INVALID_HANDLE_VALUE)
> +        {
> +          CloseHandle(file);
> +          return;
> +        }
> +    }
> +
> +   filename[0] = '\0';
> +}
> +
> +/* unhandled exception callback set with SetUnhandledExceptionFilter() */
> +LONG WINAPI
> +svn_unhandled_exception_filter(PEXCEPTION_POINTERS ptrs)
> +{
> +  char dmp_filename[MAX_PATH];
> +  char log_filename[MAX_PATH];
> +  HANDLE log_file;
> +
> +  /* Check if the crash handler was already loaded (crash while handling the
> +     crash) */
> +  if (dbghelp_dll != INVALID_HANDLE_VALUE)
> +    return EXCEPTION_CONTINUE_SEARCH;
> +
> +  /* don't log anything if we're running inside a debugger ... */
> +  if (is_debugger_present() == TRUE)
> +    return EXCEPTION_CONTINUE_SEARCH;
> +
> +  get_temp_filename(dmp_filename, "svn-crash-log", "dmp");
> +  get_temp_filename(log_filename, "svn-crash-log", "log");
Personally I prefer Application Data\Subversion for creating dump files.

> +
> +  /* ... or if we can't create the log files ... */
> +  if (*dmp_filename == '\0' || *log_filename == '\0')
> +    return EXCEPTION_CONTINUE_SEARCH;
get_temp_filename should return boolean to indicate is it right temp
names generated instead of checking for empty name.

> +
> +  /* If we can't load a recent version of the dbghelp.dll, pass on this
> +     exception */
> +  if (load_dbghelp_dll() == FALSE)
> +    return EXCEPTION_CONTINUE_SEARCH;
> +
> +  /* open log file */
> +  log_file = CreateFile(log_filename, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS,
> +                        FILE_ATTRIBUTE_NORMAL, NULL);
> +
> +  /* write information about the process */
> +  write_to_file(log_file, "\nProcess info:\n");
> +  write_process_info(ptrs ? ptrs->ExceptionRecord : NULL,
> +                     ptrs ? ptrs->ContextRecord : NULL,
> +                     log_file);
> +
> +  /* write the stacktrace, if available */
> +  write_to_file(log_file, "\nStacktrace:\n");
> +  write_stacktrace(ptrs ? ptrs->ContextRecord : NULL, log_file);
> +
> +  /* write the minidump file and use the callback to write the list of modules
> +     to the log file */
> +  write_to_file(log_file, "\n\nLoaded modules:\n");
> +  write_minidump_file(dmp_filename, ptrs,
> +                      write_module_info_callback, (void *)log_file);
> +
> +  CloseHandle(log_file);
> +
> +  cleanup_debughlp();
> +
> +  /* inform the user */
> +  fprintf(stderr, "This application has halted due to an unexpected error.\n"\
> +                  "A crash report and minidump file were saved to disk, you"\
> +                  " can find them here:\n"\
> +                  "%s\n%s\n"\
> +                  "Please send these files to %s to help us analyse\nand "\
> +                  "solve this problem.\n\n"\
> +                  "NOTE: The crash report and minidump files can contain some"\
> +                  " sensitive information\n(filenames, partial file content, "\
> +                  "usernames and passwords etc.)\n",
> +                  log_filename,
> +                  dmp_filename,
> +                  crashreport_email);
> +
> +  /* terminate the application */
> +  return EXCEPTION_EXECUTE_HANDLER;
> +}
> +#endif /* WIN32 */
> Index: subversion/libsvn_subr/win32_crashrpt.h
> ===================================================================
> --- subversion/libsvn_subr/win32_crashrpt.h     (revision 0)
> +++ subversion/libsvn_subr/win32_crashrpt.h     (revision 0)
> @@ -0,0 +1,28 @@
> +/*
> + * win32_crashrpt.h : shares the win32 crashhandler functions in libsvn_subr.
> + *
> + * ====================================================================
> + * Copyright (c) 2007 CollabNet.  All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution.  The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals.  For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifndef SVN_LIBSVN_SUBR_WIN32_CRASHRPT_H
> +#define SVN_LIBSVN_SUBR_WIN32_CRASHRPT_H
> +
> +#ifdef WIN32
> +
> +LONG WINAPI svn_unhandled_exception_filter(PEXCEPTION_POINTERS ptrs);
> +
> +#endif /* WIN32 */
> +
> +#endif /* SVN_LIBSVN_SUBR_WIN32_CRASHRPT_H */
> Index: subversion/libsvn_subr/win32_crashrpt_dll.h
> ===================================================================
> --- subversion/libsvn_subr/win32_crashrpt_dll.h (revision 0)
> +++ subversion/libsvn_subr/win32_crashrpt_dll.h (revision 0)
> @@ -0,0 +1,97 @@
> +/*
> + * win32_crashrpt_dll.h : private header file.
> + *
> + * ====================================================================
> + * Copyright (c) 2007 CollabNet.  All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution.  The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals.  For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +#ifndef SVN_LIBSVN_SUBR_WIN32_CRASHRPT_DLL_H
> +#define SVN_LIBSVN_SUBR_WIN32_CRASHRPT_DLL_H
> +
> +#ifdef WIN32
> +
> +/* public functions in dbghelp.dll */
> +typedef BOOL  (WINAPI * MINIDUMPWRITEDUMP)(HANDLE hProcess, DWORD ProcessId,
> +               HANDLE hFile, MINIDUMP_TYPE DumpType,
> +               CONST PMINIDUMP_EXCEPTION_INFORMATION ExceptionParam,
> +               CONST PMINIDUMP_USER_STREAM_INFORMATION UserStreamParam,
> +               CONST PMINIDUMP_CALLBACK_INFORMATION CallbackParam);
> +typedef BOOL  (WINAPI * SYMINITIALIZE)(HANDLE hProcess, PSTR UserSearchPath,
> +                                    BOOL fInvadeProcess);
> +typedef DWORD (WINAPI * SYMSETOPTIONS)(DWORD SymOptions);
> +
> +typedef DWORD (WINAPI * SYMGETOPTIONS)(VOID);
> +
> +typedef BOOL  (WINAPI * SYMCLEANUP)(HANDLE hProcess);
> +
> +typedef BOOL  (WINAPI * SYMGETTYPEINFO)(HANDLE hProcess, DWORD64 ModBase,
> +                                     ULONG TypeId, IMAGEHLP_SYMBOL_TYPE_INFO GetType,
> +                                     PVOID pInfo);
> +
> +typedef BOOL  (WINAPI * SYMGETLINEFROMADDR)(HANDLE hProcess, DWORD dwAddr,
> +                                 PDWORD pdwDisplacement, PIMAGEHLP_LINE Line);
> +
> +typedef BOOL  (WINAPI * SYMENUMSYMBOLS)(HANDLE hProcess, ULONG64 BaseOfDll, PCSTR Mask,
> +                             PSYM_ENUMERATESYMBOLS_CALLBACK EnumSymbolsCallback,
> +                             PVOID UserContext);
> +
> +typedef BOOL  (WINAPI * SYMSETCONTEXT)(HANDLE hProcess, PIMAGEHLP_STACK_FRAME StackFrame,
> +                            PIMAGEHLP_CONTEXT Context);
> +
> +typedef BOOL  (WINAPI * SYMFROMADDR)(HANDLE hProcess, DWORD64 Address,
> +                          PDWORD64 Displacement, PSYMBOL_INFO Symbol);
> +
> +typedef BOOL  (WINAPI * STACKWALK)(DWORD MachineType, HANDLE hProcess,
> +                                HANDLE hThread, LPSTACKFRAME StackFrame,
> +                                PVOID ContextRecord,
> +                                PREAD_PROCESS_MEMORY_ROUTINE ReadMemoryRoutine,
> +                                PFUNCTION_TABLE_ACCESS_ROUTINE FunctionTableAccessRoutine,
> +                                PGET_MODULE_BASE_ROUTINE GetModuleBaseRoutine,
> +                                PTRANSLATE_ADDRESS_ROUTINE TranslateAddress);
> +
> +typedef PVOID (WINAPI * SYMFUNCTIONTABLEACCESS)(HANDLE hProcess, DWORD AddrBase);
> +
> +typedef DWORD (WINAPI * SYMGETMODULEBASE)(HANDLE hProcess, DWORD dwAddr);
> +
> +/* public functions in version.dll */
> +typedef DWORD (APIENTRY * GETFILEVERSIONINFOSIZE)
> +                          (LPCSTR lptstrFilename, LPDWORD lpdwHandle);
> +typedef BOOL  (APIENTRY * GETFILEVERSIONINFO)
> +                          (LPCSTR lptstrFilename, DWORD dwHandle, DWORD dwLen,
> +                           LPVOID lpData);
> +typedef BOOL  (APIENTRY * VERQUERYVALUE)
> +                          (const LPVOID pBlock, LPSTR lpSubBlock,
> +                           LPVOID * lplpBuffer, PUINT puLen);
> +
> +/* public functions in kernel32.dll */
> +typedef BOOL  (WINAPI * ISDEBUGGERPRESENT)(VOID);
> +
> +/* function pointers */
> +MINIDUMPWRITEDUMP       MiniDumpWriteDump_;
> +SYMINITIALIZE           SymInitialize_;
> +SYMSETOPTIONS           SymSetOptions_;
> +SYMGETOPTIONS           SymGetOptions_;
> +SYMCLEANUP              SymCleanup_;
> +SYMGETTYPEINFO          SymGetTypeInfo_;
> +SYMGETLINEFROMADDR      SymGetLineFromAddr_;
> +SYMENUMSYMBOLS          SymEnumSymbols_;
> +SYMSETCONTEXT           SymSetContext_;
> +SYMFROMADDR             SymFromAddr_;
> +STACKWALK               StackWalk_;
> +SYMFUNCTIONTABLEACCESS  SymFunctionTableAccess_;
> +SYMGETMODULEBASE        SymGetModuleBase_;
> +
> +#endif /* WIN32 */
> +
> +#endif /* SVN_LIBSVN_SUBR_WIN32_CRASHRPT_DLL_H */
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>


-- 
Ivan Zhakov

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org