* [PATCH] xmalloc: include size in the failure message @ 2010-08-20 13:01 Matthieu Moy 2010-08-20 14:47 ` Junio C Hamano 0 siblings, 1 reply; 4+ messages in thread From: Matthieu Moy @ 2010-08-20 13:01 UTC (permalink / raw) To: git, gitster; +Cc: Matthieu Moy 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(-) 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xmalloc: include size in the failure message 2010-08-20 13:01 [PATCH] xmalloc: include size in the failure message Matthieu Moy @ 2010-08-20 14:47 ` Junio C Hamano 2010-08-20 15:09 ` Matthieu Moy 0 siblings, 1 reply; 4+ messages in thread From: Junio C Hamano @ 2010-08-20 14:47 UTC (permalink / raw) To: Matthieu Moy; +Cc: git 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. But why not. > 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(-) 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) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] xmalloc: include size in the failure message 2010-08-20 14:47 ` Junio C Hamano @ 2010-08-20 15:09 ` Matthieu Moy 2010-08-20 16:31 ` Junio C Hamano 0 siblings, 1 reply; 4+ messages in thread From: Matthieu Moy @ 2010-08-20 15:09 UTC (permalink / raw) To: git, gitster; +Cc: Matthieu Moy 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: > 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. >> 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. > 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(-) 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] xmalloc: include size in the failure message 2010-08-20 15:09 ` Matthieu Moy @ 2010-08-20 16:31 ` Junio C Hamano 0 siblings, 0 replies; 4+ messages in thread From: Junio C Hamano @ 2010-08-20 16:31 UTC (permalink / raw) To: Matthieu Moy; +Cc: git Matthieu Moy <Matthieu.Moy@imag.fr> writes: >> 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? ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-08-20 16:32 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-08-20 13:01 [PATCH] xmalloc: include size in the failure message Matthieu Moy 2010-08-20 14:47 ` Junio C Hamano 2010-08-20 15:09 ` Matthieu Moy 2010-08-20 16:31 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).