{"thread":{"id":"24804","subject":"[PATCH] xmalloc: include size in the failure message","startedAt":"2010-08-20T13:01:12Z","lastAt":"2010-08-20T16:31:44Z","messageCount":4,"participants":["Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"148557","messageId":"1282309272-20906-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24804","inReplyTo":null,"subject":"[PATCH] xmalloc: include size in the failure message","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-20T13:01:12Z","receivedAt":"2010-08-20T13:01:12Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Out-of-memory errors can either be actual lack of memory, or bugs (like\ncode trying to call xmalloc(-1) by mistake). A little more information\nmay help tracking bugs reported by users.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nThis kind of thing may help for cases like\n\nSubject: Out of memory error during git push\nhttp://thread.gmane.org/gmane.comp.version-control.git/153988\n\n wrapper.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex afb4f6f..7057cbd 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -40,7 +40,7 @@ void *xmalloc(size_t size)\n \t\tif (!ret && !size)\n \t\t\tret = malloc(1);\n \t\tif (!ret)\n-\t\t\tdie(\"Out of memory, malloc failed\");\n+\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %u bytes)\", size);\n \t}\n #ifdef XMALLOC_POISON\n \tmemset(ret, 0xA5, size);\n-- \n1.7.2.1.83.ge0227\n"},{"id":"148573","messageId":"7vhbipcpe1.fsf@alter.siamese.dyndns.org","threadId":"24804","inReplyTo":"1282309272-20906-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] xmalloc: include size in the failure message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-20T14:47:18Z","receivedAt":"2010-08-20T14:47:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n> Out-of-memory errors can either be actual lack of memory, or bugs (like\n> code trying to call xmalloc(-1) by mistake). A little more information\n> may help tracking bugs reported by users.\n>\n> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n> ---\n> This kind of thing may help for cases like\n>\n> Subject: Out of memory error during git push\n> http://thread.gmane.org/gmane.comp.version-control.git/153988\n\nUnless a single allocation try to grab unreasonably amount of memory,\nprobably a failure from a specific single failure may not help much.\n\nBut why not.\n\n>  wrapper.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/wrapper.c b/wrapper.c\n> index afb4f6f..7057cbd 100644\n> --- a/wrapper.c\n> +++ b/wrapper.c\n> @@ -40,7 +40,7 @@ void *xmalloc(size_t size)\n>  \t\tif (!ret && !size)\n>  \t\t\tret = malloc(1);\n>  \t\tif (!ret)\n> -\t\t\tdie(\"Out of memory, malloc failed\");\n> +\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %u bytes)\", size);\n\nPerhaps use %lu format with cast to ulong?\n\nI see (conditional) use of %zu in alloc.c only for a debugging codepath\nnobody exercises, which does this:\n\n        #ifdef NO_C99_FORMAT\n        #define SZ_FMT \"%u\"\n        #else\n        #define SZ_FMT \"%zu\"\n        #endif\n\n        static void report(const char *name, unsigned int count, size_t size)\n        {\n            fprintf(stderr, \"%10s: %8u (\" SZ_FMT \" kB)\\n\", name, count, size);\n        }\n\nwhich looks wrong.\n\n-- >8 --\nalloc.c: fix formatting size_t to string\n\nUnder NO_C99_FORMAT the format and the argument would not match if size_t\nis not the same size as uint.  As the one in sha1_file.c seems to be done\nin a better way, let's use that one.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n alloc.c     |   11 ++---------\n cache.h     |    8 ++++++++\n sha1_file.c |    8 --------\n 3 files changed, 10 insertions(+), 17 deletions(-)\n\ndiff --git a/alloc.c b/alloc.c\nindex 6ef6753..5324115 100644\n--- a/alloc.c\n+++ b/alloc.c\n@@ -51,19 +51,12 @@ DEFINE_ALLOCATOR(commit, struct commit)\n DEFINE_ALLOCATOR(tag, struct tag)\n DEFINE_ALLOCATOR(object, union any_object)\n \n-#ifdef NO_C99_FORMAT\n-#define SZ_FMT \"%u\"\n-#else\n-#define SZ_FMT \"%zu\"\n-#endif\n-\n static void report(const char *name, unsigned int count, size_t size)\n {\n-    fprintf(stderr, \"%10s: %8u (\" SZ_FMT \" kB)\\n\", name, count, size);\n+    fprintf(stderr, \"%10s: %8u (%\" SZ_FMT \" kB)\\n\", name, count,\n+\t    sz_fmt(size));\n }\n \n-#undef SZ_FMT\n-\n #define REPORT(name)\t\\\n     report(#name, name##_allocs, name##_allocs*sizeof(struct name) >> 10)\n \ndiff --git a/cache.h b/cache.h\nindex 37ef9d8..1cfc5f0 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1101,4 +1101,12 @@ int split_cmdline(char *cmdline, const char ***argv);\n /* builtin/merge.c */\n int checkout_fast_forward(const unsigned char *from, const unsigned char *to);\n \n+#ifdef NO_C99_FORMAT\n+#define SZ_FMT \"lu\"\n+static inline unsigned long sz_fmt(size_t s) { return (unsigned long)s; }\n+#else\n+#define SZ_FMT \"zu\"\n+static inline size_t sz_fmt(size_t s) { return s; }\n+#endif\n+\n #endif /* CACHE_H */\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 0cd9435..4f392b9 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -25,14 +25,6 @@\n #endif\n #endif\n \n-#ifdef NO_C99_FORMAT\n-#define SZ_FMT \"lu\"\n-static unsigned long sz_fmt(size_t s) { return (unsigned long)s; }\n-#else\n-#define SZ_FMT \"zu\"\n-static size_t sz_fmt(size_t s) { return s; }\n-#endif\n-\n const unsigned char null_sha1[20];\n \n int safe_create_leading_directories(char *path)\n"},{"id":"148576","messageId":"1282316991-23320-1-git-send-email-Matthieu.Moy@imag.fr","threadId":"24804","inReplyTo":"7vhbipcpe1.fsf@alter.siamese.dyndns.org","subject":"[PATCH] xmalloc: include size in the failure message","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2010-08-20T15:09:51Z","receivedAt":"2010-08-20T15:09:51Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Out-of-memory errors can either be actual lack of memory, or bugs (like\ncode trying to call xmalloc(-1) by mistake). A little more information\nmay help tracking bugs reported by users.\n\nSigned-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n---\nJunio C Hamano <gitster@pobox.com> writes:\n\n> Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n>\n>> Out-of-memory errors can either be actual lack of memory, or bugs (like\n>> code trying to call xmalloc(-1) by mistake). A little more information\n>> may help tracking bugs reported by users.\n>>\n>> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>\n>> ---\n>> This kind of thing may help for cases like\n>>\n>> Subject: Out of memory error during git push\n>> http://thread.gmane.org/gmane.comp.version-control.git/153988\n>\n> Unless a single allocation try to grab unreasonably amount of memory,\n> probably a failure from a specific single failure may not help much.\n\nI don't promise miracle ;-). But at least, the diagnosis allows one to\ndistinguish small allocations, huge ones, and silly errors like small\nnegative numbers turned into unreasonably big ones.\n\n>>  \t\tif (!ret)\n>> -\t\t\tdie(\"Out of memory, malloc failed\");\n>> +\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %u bytes)\", size);\n>\n> Perhaps use %lu format with cast to ulong?\n\nWill do.\n\n> alloc.c: fix formatting size_t to string\n>\n> Under NO_C99_FORMAT the format and the argument would not match if size_t\n> is not the same size as uint.  As the one in sha1_file.c seems to be done\n> in a better way, let's use that one.\n\nSounds good. Not sure why we don't basically use the %lu version\neverywhere though.\n\n wrapper.c |    3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/wrapper.c b/wrapper.c\nindex afb4f6f..fd8ead3 100644\n--- a/wrapper.c\n+++ b/wrapper.c\n@@ -40,7 +40,8 @@ void *xmalloc(size_t size)\n \t\tif (!ret && !size)\n \t\t\tret = malloc(1);\n \t\tif (!ret)\n-\t\t\tdie(\"Out of memory, malloc failed\");\n+\t\t\tdie(\"Out of memory, malloc failed (tried to allocate %lu bytes)\",\n+\t\t\t    (unsigned long)size);\n \t}\n #ifdef XMALLOC_POISON\n \tmemset(ret, 0xA5, size);\n-- \n1.7.2.1.83.ge0227\n"},{"id":"148579","messageId":"7vd3tdckjz.fsf@alter.siamese.dyndns.org","threadId":"24804","inReplyTo":"1282316991-23320-1-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] xmalloc: include size in the failure message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-08-20T16:31:44Z","receivedAt":"2010-08-20T16:31:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy <Matthieu.Moy@imag.fr> writes:\n\n>> Under NO_C99_FORMAT the format and the argument would not match if size_t\n>> is not the same size as uint.  As the one in sha1_file.c seems to be done\n>> in a better way, let's use that one.\n>\n> Sounds good. Not sure why we don't basically use the %lu version\n> everywhere though.\n\nI have been wondering about the same thing, but perhaps because we won't\nhave to worry too much about size_t needing to be unsigned long long on\nolder platforms where %z is not supported, while we expect %z will be\navailable on larger where using %lu may become an issue?\n"}]}