Re: [PATCH] minidumper patches

2014-04-21 Thread Christopher Faylor
On Mon, Apr 21, 2014 at 06:02:19PM +0100, Jon TURNEY wrote:
>
>Attached are a couple of patches for the minidumper utility which could
>probably use some review.

This is your utility so, as far as I'm concerned, you have carte blanche
to check things in, i.e., you don't need any approval.  I didn't see anything
worth commenting on other than a few GNU coding issues, i.e., spaces
before parentheses for functions.

cgf


[PATCH] minidumper patches

2014-04-21 Thread Jon TURNEY

Attached are a couple of patches for the minidumper utility which could
probably use some review.

The first one changes to direct linkage with dbghelp, rather than using
GetProcAddress().

(This seems to be the current style, I assume since the reasons for not
directly linking (no import libs or Windows versions which don't have the
.dll) are no longer relevant.)

As suggested, the second one changes the default dump type to something with
more information that just MiniDumpNormal, without getting too big.

(This is complicated by the fact that it appears to be an error to call
MiniDumpWriteDump() with an unknown dump flag, but the documentation for which
dump flags are supported by which dbhelp.dll version and how
ImagehlpApiVersionEx() is supposed to work is not totally clear)
Link directly with dbghelp

2014-04-21  Jon TURNEY  

* Makefile.in (minidumper.exe): Link directly with dbghelp.
* minidumper.cc (minidump): Ditto.

---
 utils/Makefile.in   |1 +
 utils/minidumper.cc |   47 ++-
 2 files changed, 11 insertions(+), 37 deletions(-)

Index: winsup/utils/Makefile.in
===
--- winsup.orig/utils/Makefile.in
+++ winsup/utils/Makefile.in
@@ -96,6 +96,7 @@ strace.exe: MINGW_LDFLAGS += -lntdll
 
 ldd.exe:CYGWIN_LDFLAGS += -lpsapi
 pldd.exe: CYGWIN_LDFLAGS += -lpsapi
+minidumper.exe: CYGWIN_LDFLAGS += -ldbghelp
 
 ldh.exe: MINGW_LDFLAGS += -nostdlib -lkernel32
 
Index: winsup/utils/minidumper.cc
===
--- winsup.orig/utils/minidumper.cc
+++ winsup/utils/minidumper.cc
@@ -26,42 +26,16 @@
 #include 
 #include 
 #include 
+#include 
 
 BOOL verbose = FALSE;
 BOOL nokill = FALSE;
 
-typedef DWORD MINIDUMP_TYPE;
-
-typedef BOOL (WINAPI *MiniDumpWriteDump_type)(
-  HANDLE hProcess,
-  DWORD dwPid,
-  HANDLE hFile,
-  MINIDUMP_TYPE DumpType,
-  CONST void *ExceptionParam,
-  CONST void *UserStreamParam,
-  CONST void *allbackParam);
-
 static void
 minidump(DWORD pid, MINIDUMP_TYPE dump_type, const char *minidump_file)
 {
   HANDLE dump_file;
   HANDLE process;
-  MiniDumpWriteDump_type MiniDumpWriteDump_fp;
-  HMODULE module;
-
-  module = LoadLibrary("dbghelp.dll");
-  if (!module)
-{
-  fprintf (stderr, "error loading DbgHelp\n");
-  return;
-}
-
-  MiniDumpWriteDump_fp = (MiniDumpWriteDump_type)GetProcAddress(module, 
"MiniDumpWriteDump");
-  if (!MiniDumpWriteDump_fp)
-{
-  fprintf (stderr, "error getting the address of MiniDumpWriteDump\n");
-  return;
-}
 
   dump_file = CreateFile(minidump_file,
  GENERIC_READ | GENERIC_WRITE,
@@ -85,13 +59,13 @@ minidump(DWORD pid, MINIDUMP_TYPE dump_t
   return;
 }
 
-  BOOL success = (*MiniDumpWriteDump_fp)(process,
- pid,
- dump_file,
- dump_type,
- NULL,
- NULL,
- NULL);
+  BOOL success = MiniDumpWriteDump(process,
+   pid,
+   dump_file,
+   dump_type,
+   NULL,
+   NULL,
+   NULL);
   if (success)
 {
   if (verbose)
@@ -112,7 +86,6 @@ minidump(DWORD pid, MINIDUMP_TYPE dump_t
 
   CloseHandle(process);
   CloseHandle(dump_file);
-  FreeLibrary(module);
 }
 
 static void
@@ -164,7 +137,7 @@ main (int argc, char **argv)
   int opt;
   const char *p = "";
   DWORD pid;
-  MINIDUMP_TYPE dump_type = 0; // MINIDUMP_NORMAL
+  MINIDUMP_TYPE dump_type = MiniDumpNormal;
 
   while ((opt = getopt_long (argc, argv, opts, longopts, NULL) ) != EOF)
 switch (opt)
@@ -172,7 +145,7 @@ main (int argc, char **argv)
   case 't':
 {
   char *endptr;
-  dump_type = strtoul(optarg, &endptr, 0);
+  dump_type = (MINIDUMP_TYPE)strtoul(optarg, &endptr, 0);
   if (*endptr != '\0')
 {
   fprintf (stderr, "syntax error in minidump type \"%s\" near 
character #%d.\n", optarg, (int) (endptr - optarg));
Change default dump type from MiniDumpNormal to something with more useful 
information without getting too big.

2014-04-21  Jon TURNEY  

* minidumper.cc (filter_minidump_type): New function.
(minidump): Change default dump type from MiniDumpNormal to
something with more useful information without getting too
big. U