All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: [PATCH 5/7] Introduce malloc/strdup/pathdup variants for permanent allocations
Date: Sat, 2 Oct 2010 03:38:51 -0500	[thread overview]
Message-ID: <20101002083851.GF29638@burratino> (raw)
In-Reply-To: <20101002082752.GA29638@burratino>

In git (option parsing and some other places), we sometimes leak tiny
bits of memory on purpose.  Introduce a way to declare such
allocations, to make the lives of people searching for real memory
leaks easier.

Currently these are thin wrappers around xmalloc and xstrdup.  The
KEEP_WRAPPER attribute avoids a tail call to ensure their stack frame
does not get replaced with the true xmalloc/xstrdup, so tools like
valgrind can easily distinguish them and know that the memory they
return is not meant to be freed.

In the long run it might be nice to tighten the semantics of these
functions and use memory from a large pool that is never freed (to
avoid fragmentation and save some allocation overhead).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 cache.h                 |    2 ++
 git-compat-util.h       |    2 ++
 path.c                  |   10 ++++++++++
 t/valgrind/default.supp |   17 +++++++++++++++++
 wrapper.c               |   14 ++++++++++++++
 5 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 9ee61a2..bbf32ff 100644
--- a/cache.h
+++ b/cache.h
@@ -638,6 +638,8 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
+extern char *git_pathdup_permanent(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/git-compat-util.h b/git-compat-util.h
index 81883e7..f00ee33 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -380,7 +380,9 @@ typedef void (*try_to_free_t)(size_t);
 extern try_to_free_t set_try_to_free_routine(try_to_free_t);
 
 extern char *xstrdup(const char *str);
+extern char *strdup_permanent(const char *str);
 extern void *xmalloc(size_t size);
+extern void *alloc_permanent(size_t size);
 extern void *xmallocz(size_t size);
 extern void *xmemdupz(const void *data, size_t len);
 extern char *xstrndup(const char *str, size_t len);
diff --git a/path.c b/path.c
index a2c9d1e..332c903 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,16 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(path);
 }
 
+char *git_pathdup_permanent(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	va_start(args, fmt);
+	(void)git_vsnpath(path, sizeof(path), fmt, args);
+	va_end(args);
+	return strdup_permanent(path);
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
diff --git a/t/valgrind/default.supp b/t/valgrind/default.supp
index 7789edd..d237210 100644
--- a/t/valgrind/default.supp
+++ b/t/valgrind/default.supp
@@ -45,6 +45,23 @@
 }
 
 {
+	leaky malloc for option parsing and configuration
+	Memcheck:Leak
+	fun:malloc
+	fun:xmalloc
+	fun:alloc_permanent
+}
+
+{
+	leaky strdup for option parsing and configuration
+	Memcheck:Leak
+	fun:malloc
+	fun:strdup
+	fun:xstrdup
+	fun:strdup_permanent
+}
+
+{
 	see api-lockfile.txt
 	Memcheck:Leak
 	fun:calloc
diff --git a/wrapper.c b/wrapper.c
index fd8ead3..88ca3dd 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -3,6 +3,8 @@
  */
 #include "cache.h"
 
+#define KEEP_WRAPPER __attribute__((optimize("-fno-optimize-sibling-calls")))
+
 static void try_to_free_builtin(size_t size)
 {
 	release_pack_memory(size, -1);
@@ -17,6 +19,12 @@ try_to_free_t set_try_to_free_routine(try_to_free_t routine)
 	return old;
 }
 
+KEEP_WRAPPER
+char *strdup_permanent(const char *str)
+{
+	return xstrdup(str);
+}
+
 char *xstrdup(const char *str)
 {
 	char *ret = strdup(str);
@@ -29,6 +37,12 @@ char *xstrdup(const char *str)
 	return ret;
 }
 
+KEEP_WRAPPER
+void *alloc_permanent(size_t size)
+{
+	return xmalloc(size);
+}
+
 void *xmalloc(size_t size)
 {
 	void *ret = malloc(size);
-- 
1.7.2.3

  parent reply	other threads:[~2010-10-02  8:42 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-09  9:48 [PATCH] memory leak reported by valgrind Ivan Kanis
2010-08-09 19:37 ` Junio C Hamano
2010-08-09 20:19   ` Erik Faye-Lund
2010-08-09 20:29     ` Alex Riesen
2010-08-10  3:26   ` Jonathan Nieder
2010-08-10  3:28     ` [PATCH/RFC 1/3] core: Stop leaking ondisk_cache_entrys Jonathan Nieder
2010-08-10  3:32     ` [PATCH/RFC 2/3] write-tree: Avoid leak when index refers to an invalid object Jonathan Nieder
2010-08-10  3:33     ` [PATCH/RFC 3/3] read-tree: stop leaking tree objects Jonathan Nieder
2010-10-02  8:27     ` [PATCH/RFC 0/7] Re: [PATCH] memory leak reported by valgrind Jonathan Nieder
2010-10-02  8:31       ` [PATCH 1/7] init: plug tiny one-time memory leak Jonathan Nieder
2010-10-03 23:46         ` Junio C Hamano
2010-10-04  4:34           ` [PATCH v2] " Jonathan Nieder
2010-10-02  8:32       ` [PATCH 2/7] lockfile: introduce alloc_lock_file() to avoid valgrind noise Jonathan Nieder
2010-10-02 16:30         ` Andreas Ericsson
2010-10-02 16:43           ` Jonathan Nieder
2010-10-06 20:10         ` Junio C Hamano
2010-10-06 20:21           ` Jonathan Nieder
2010-10-06 21:45             ` Junio C Hamano
2010-10-06 22:17               ` Jonathan Nieder
2010-10-02  8:35       ` [PATCH 3/7] environment.c: remove unused variable Jonathan Nieder
2010-10-02  8:36       ` [PATCH 4/7] setup: make sure git dir path is in a permanent buffer Jonathan Nieder
2010-10-04  9:25         ` Nguyen Thai Ngoc Duy
2010-10-02  8:38       ` Jonathan Nieder [this message]
2010-10-02  8:39       ` [PATCH 6/7] environment: use alloc_permanent() for computed git_dir and co Jonathan Nieder
2010-10-02  8:41       ` [PATCH 7/7] commit-tree: free commit message before exiting Jonathan Nieder
2010-10-02 18:14         ` Sverre Rabbelier
2010-10-02 18:26           ` Jonathan Nieder
2010-10-02 20:12             ` Sverre Rabbelier
2010-10-02 20:50               ` Jonathan Nieder
2010-10-04  0:31         ` Junio C Hamano
2010-10-04  4:18           ` Jonathan Nieder
2010-10-04  7:55             ` Junio C Hamano
2010-10-04  7:56               ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101002083851.GF29638@burratino \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.