Commit 827b954ef400f3c488500ad6c8a57246223dde9c

Jeff Hostetler 2015-06-28T06:56:02

Reserve aux_id 0; sort leaks by aux_id. Fix cmp.

diff --git a/src/win32/w32_stack.c b/src/win32/w32_stack.c
index f1e5895..15af3dc 100644
--- a/src/win32/w32_stack.c
+++ b/src/win32/w32_stack.c
@@ -76,7 +76,7 @@ int git_win32__stack_compare(
 	git_win32__stack__raw_data *d1,
 	git_win32__stack__raw_data *d2)
 {
-	return memcmp(d1, d2, sizeof(d1));
+	return memcmp(d1, d2, sizeof(*d1));
 }
 
 int git_win32__stack_format(
@@ -88,7 +88,6 @@ int git_win32__stack_format(
 
 	/* SYMBOL_INFO has char FileName[1] at the end.  The docs say to
 	 * to malloc it with extra space for your desired max filename.
-	 * We use a union to do the same thing without mallocing.
 	 */
 	struct {
 		SYMBOL_INFO symbol;
@@ -152,12 +151,25 @@ int git_win32__stack_format(
 		buf_used += detail_len;
 	}
 
-	/* If an "aux" data provider was registered, ask it to append its detailed
-	 * data to the end of ours using the "aux_id" it gave us when this de-duped
-	 * item was created.
+	/* "aux_id" 0 is reserved to mean no aux data. This is needed to handle
+	 * allocs that occur before the aux callbacks were registered.
 	 */
-	if (g_aux_cb_lookup)
-		(g_aux_cb_lookup)(pdata->aux_id, &pbuf[buf_used], (buf_len - buf_used - 1));
+	if (pdata->aux_id > 0) {
+		p_snprintf(detail, sizeof(detail), "%saux_id: %d%s",
+				   prefix, pdata->aux_id, suffix);
+		detail_len = strlen(detail);
+		if ((buf_used + detail_len + 1) < buf_len) {
+			memcpy(&pbuf[buf_used], detail, detail_len);
+			buf_used += detail_len;
+		}
+
+		/* If an "aux" data provider is still registered, ask it to append its detailed
+		 * data to the end of ours using the "aux_id" it gave us when this de-duped
+		 * item was created.
+		 */
+		if (g_aux_cb_lookup)
+			(g_aux_cb_lookup)(pdata->aux_id, &pbuf[buf_used], (buf_len - buf_used - 1));
+	}
 
 	return GIT_OK;
 }
diff --git a/src/win32/w32_stack.h b/src/win32/w32_stack.h
index b457d2e..21170bd 100644
--- a/src/win32/w32_stack.h
+++ b/src/win32/w32_stack.h
@@ -19,7 +19,8 @@
  * This callback will be called during crtdbg-instrumented allocs.
  *
  * @param aux_id [out] A returned "aux_id" representing a unique
- * (de-duped at the C# layer) stacktrace.
+ * (de-duped at the C# layer) stacktrace.  "aux_id" 0 is reserved
+ * to mean no aux stacktrace data.
  */
 typedef void (*git_win32__stack__aux_cb_alloc)(unsigned int *aux_id);
 
@@ -60,11 +61,16 @@ GIT_EXTERN(int) git_win32__stack__set_aux_cb(
 /**
  * Wrapper containing the raw unprocessed stackframe
  * data for a single stacktrace and any "aux_id".
+ *
+ * I put the aux_id first so leaks will be sorted by it.
+ * So, for example, if a specific callstack in C# leaks
+ * a repo handle, all of the pointers within the associated
+ * repo pointer will be grouped together.
  */
 typedef struct {
-	void *frames[GIT_WIN32__STACK__MAX_FRAMES];
-	unsigned int nr_frames;
 	unsigned int aux_id;
+	unsigned int nr_frames;
+	void *frames[GIT_WIN32__STACK__MAX_FRAMES];
 } git_win32__stack__raw_data;