threads / patch / 24804

patchxmalloc: include size in the failure message

Subject: [PATCH] xmalloc: include size in the failure message

## tl;dr

4 messages between Aug 20, 2010 and Aug 20, 2010. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Matthieu Moy· Aug 20, 2010, 13:01 UTC · lore

Out-of-memory errors can either be actual lack of memory, or bugs (like code trying to call xmalloc(-1) by mistake). A little more information may help tracking bugs reported by users.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
This kind of thing may help for cases like
Subject: Out of memory error during git push
http://thread.gmane.org/gmane.comp.version-control.git/153988
 wrapper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Show changes to wrapper.c +1 −1
diff --git a/wrapper.c b/wrapper.c
index afb4f6f..7057cbd 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -40,7 +40,7 @@ void *xmalloc(size_t size)
 		if (!ret && !size)
 			ret = malloc(1);
 		if (!ret)
-			die("Out of memory, malloc failed");
+			die("Out of memory, malloc failed (tried to allocate %u bytes)", size);
 	}
 #ifdef XMALLOC_POISON
 	memset(ret, 0xA5, size);
-- 
1.7.2.1.83.ge0227
Junio C Hamano· Aug 20, 2010, 14:47 UTC · re: Matthieu Moy · lore

Re: [PATCH] xmalloc: include size in the failure message

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 10 quoted lines
> Out-of-memory errors can either be actual lack of memory, or bugs (like
> code trying to call xmalloc(-1) by mistake). A little more information
> may help tracking bugs reported by users.
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> This kind of thing may help for cases like
>
> Subject: Out of memory error during git push
> http://thread.gmane.org/gmane.comp.version-control.git/153988

Unless a single allocation try to grab unreasonably amount of memory, probably a failure from a specific single failure may not help much.

But why not.
Show 13 quoted lines
>  wrapper.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/wrapper.c b/wrapper.c
> index afb4f6f..7057cbd 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -40,7 +40,7 @@ void *xmalloc(size_t size)
>  		if (!ret && !size)
>  			ret = malloc(1);
>  		if (!ret)
> -			die("Out of memory, malloc failed");
> +			die("Out of memory, malloc failed (tried to allocate %u bytes)", size);
Perhaps use %lu format with cast to ulong?

I see (conditional) use of %zu in alloc.c only for a debugging codepath nobody exercises, which does this:

        #ifdef NO_C99_FORMAT
        #define SZ_FMT "%u"
        #else
        #define SZ_FMT "%zu"
        #endif
        static void report(const char *name, unsigned int count, size_t size)
        {
            fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)\n", name, count, size);
        }
which looks wrong.

-- >8 -- alloc.c: fix formatting size_t to string

Under NO_C99_FORMAT the format and the argument would not match if size_t is not the same size as uint. As the one in sha1_file.c seems to be done in a better way, let's use that one.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 alloc.c     |   11 ++---------
 cache.h     |    8 ++++++++
 sha1_file.c |    8 --------
 3 files changed, 10 insertions(+), 17 deletions(-)
Show changes to 3 files +10 −17

alloc.c, cache.h, sha1_file.c

diff --git a/alloc.c b/alloc.c
index 6ef6753..5324115 100644
--- a/alloc.c
+++ b/alloc.c
@@ -51,19 +51,12 @@ DEFINE_ALLOCATOR(commit, struct commit)
 DEFINE_ALLOCATOR(tag, struct tag)
 DEFINE_ALLOCATOR(object, union any_object)
 
-#ifdef NO_C99_FORMAT
-#define SZ_FMT "%u"
-#else
-#define SZ_FMT "%zu"
-#endif
-
 static void report(const char *name, unsigned int count, size_t size)
 {
-    fprintf(stderr, "%10s: %8u (" SZ_FMT " kB)\n", name, count, size);
+    fprintf(stderr, "%10s: %8u (%" SZ_FMT " kB)\n", name, count,
+	    sz_fmt(size));
 }
 
-#undef SZ_FMT
-
 #define REPORT(name)	\
     report(#name, name##_allocs, name##_allocs*sizeof(struct name) >> 10)
 
diff --git a/cache.h b/cache.h
index 37ef9d8..1cfc5f0 100644
--- a/cache.h
+++ b/cache.h
@@ -1101,4 +1101,12 @@ int split_cmdline(char *cmdline, const char ***argv);
 /* builtin/merge.c */
 int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
 
+#ifdef NO_C99_FORMAT
+#define SZ_FMT "lu"
+static inline unsigned long sz_fmt(size_t s) { return (unsigned long)s; }
+#else
+#define SZ_FMT "zu"
+static inline size_t sz_fmt(size_t s) { return s; }
+#endif
+
 #endif /* CACHE_H */
diff --git a/sha1_file.c b/sha1_file.c
index 0cd9435..4f392b9 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -25,14 +25,6 @@
 #endif
 #endif
 
-#ifdef NO_C99_FORMAT
-#define SZ_FMT "lu"
-static unsigned long sz_fmt(size_t s) { return (unsigned long)s; }
-#else
-#define SZ_FMT "zu"
-static size_t sz_fmt(size_t s) { return s; }
-#endif
-
 const unsigned char null_sha1[20];
 
 int safe_create_leading_directories(char *path)
Matthieu Moy· Aug 20, 2010, 15:09 UTC · re: Junio C Hamano · lore

Out-of-memory errors can either be actual lack of memory, or bugs (like code trying to call xmalloc(-1) by mistake). A little more information may help tracking bugs reported by users.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Out-of-memory errors can either be actual lack of memory, or bugs (like
>> code trying to call xmalloc(-1) by mistake). A little more information
>> may help tracking bugs reported by users.
>>
>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>> ---
>> This kind of thing may help for cases like
>>
>> Subject: Out of memory error during git push
>> http://thread.gmane.org/gmane.comp.version-control.git/153988
>
> Unless a single allocation try to grab unreasonably amount of memory,
> probably a failure from a specific single failure may not help much.

I don't promise miracle ;-). But at least, the diagnosis allows one to distinguish small allocations, huge ones, and silly errors like small negative numbers turned into unreasonably big ones.

Show 5 quoted lines
>>  		if (!ret)
>> -			die("Out of memory, malloc failed");
>> +			die("Out of memory, malloc failed (tried to allocate %u bytes)", size);
>
> Perhaps use %lu format with cast to ulong?
Will do.
Show 5 quoted lines
> alloc.c: fix formatting size_t to string
>
> Under NO_C99_FORMAT the format and the argument would not match if size_t
> is not the same size as uint.  As the one in sha1_file.c seems to be done
> in a better way, let's use that one.

Sounds good. Not sure why we don't basically use the %lu version everywhere though.

 wrapper.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Show changes to wrapper.c +2 −1
diff --git a/wrapper.c b/wrapper.c
index afb4f6f..fd8ead3 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -40,7 +40,8 @@ void *xmalloc(size_t size)
 		if (!ret && !size)
 			ret = malloc(1);
 		if (!ret)
-			die("Out of memory, malloc failed");
+			die("Out of memory, malloc failed (tried to allocate %lu bytes)",
+			    (unsigned long)size);
 	}
 #ifdef XMALLOC_POISON
 	memset(ret, 0xA5, size);
-- 
1.7.2.1.83.ge0227
Junio C Hamano· Aug 20, 2010, 16:31 UTC · re: Matthieu Moy · lore

Re: [PATCH] xmalloc: include size in the failure message

Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 6 quoted lines
>> Under NO_C99_FORMAT the format and the argument would not match if size_t
>> is not the same size as uint.  As the one in sha1_file.c seems to be done
>> in a better way, let's use that one.
>
> Sounds good. Not sure why we don't basically use the %lu version
> everywhere though.

I have been wondering about the same thing, but perhaps because we won't have to worry too much about size_t needing to be unsigned long long on older platforms where %z is not supported, while we expect %z will be available on larger where using %lu may become an issue?

← back to recent threads