I like this a lot. There are some trivial differences in the various xmalloc.h as well, and I think you could make the style consistent within the files (eg "return i" in xasprintf and xsnprintf).
On Thu, Nov 05, 2015 at 03:50:29PM +0100, Tobias Stoeckmann wrote: > On Thu, Nov 05, 2015 at 09:50:48AM +0000, Nicholas Marriott wrote: > > I don't know why cvs and rcs xmalloc.c has ended up so different. > > It's not just about cvs and rcs: > > /usr/src/usr.bin/cvs/xmalloc.c > /usr/src/usr.bin/diff/xmalloc.c > /usr/src/usr.bin/file/xmalloc.c > /usr/src/usr.bin/rcs/xmalloc.c > /usr/src/usr.bin/ssh/xmalloc.c > /usr/src/usr.bin/tmux/xmalloc.c (probably not same origin) > > All of them share code parts that almost look identical. Some of them > skip tests, do additional tests, test for other return values, or have > typos in their error messages (or call err instead of errx, duplicating > their messages). > > This diff would unify them, taking into account that still different > style guides apply (tmux) and some use fatal() or errx() with even > different return values (diff). Ugh... > > > Index: usr.bin/cvs/xmalloc.c > =================================================================== > RCS file: /cvs/src/usr.bin/cvs/xmalloc.c,v > retrieving revision 1.12 > diff -u -p -u -p -r1.12 xmalloc.c > --- usr.bin/cvs/xmalloc.c 5 Nov 2015 09:48:21 -0000 1.12 > +++ usr.bin/cvs/xmalloc.c 5 Nov 2015 14:42:09 -0000 > @@ -13,6 +13,7 @@ > * called by a name other than "ssh" or "Secure Shell". > */ > > +#include <limits.h> > #include <stdint.h> > #include <stdio.h> > #include <stdlib.h> > @@ -30,7 +31,7 @@ xmalloc(size_t size) > fatal("xmalloc: zero size"); > ptr = malloc(size); > if (ptr == NULL) > - fatal("xmalloc: out of memory (allocating %lu bytes)", (u_long) > size); > + fatal("xmalloc: out of memory (allocating %zu bytes)", size); > return ptr; > } > > @@ -41,12 +42,10 @@ xcalloc(size_t nmemb, size_t size) > > if (size == 0 || nmemb == 0) > fatal("xcalloc: zero size"); > - if (SIZE_MAX / nmemb < size) > - fatal("xcalloc: nmemb * size > SIZE_MAX"); > ptr = calloc(nmemb, size); > if (ptr == NULL) > - fatal("xcalloc: out of memory (allocating %lu bytes)", > - (u_long)(size * nmemb)); > + fatal("xcalloc: out of memory (allocating %zu * %zu bytes)", > + nmemb, size); > return ptr; > } > > @@ -54,28 +53,23 @@ void * > xreallocarray(void *ptr, size_t nmemb, size_t size) > { > void *new_ptr; > - size_t new_size = nmemb * size; > > - if (new_size == 0) > - fatal("xrealloc: zero size"); > - if (SIZE_MAX / nmemb < size) > - fatal("xrealloc: nmemb * size > SIZE_MAX"); > - new_ptr = realloc(ptr, new_size); > + if (nmemb == 0 || size == 0) > + fatal("xreallocarray: zero size"); > + new_ptr = reallocarray(ptr, nmemb, size); > if (new_ptr == NULL) > - fatal("xrealloc: out of memory (new_size %lu bytes)", > - (u_long) new_size); > + fatal("xreallocarray: out of memory " > + "(allocating %zu * %zu bytes)", nmemb, size); > return new_ptr; > } > > char * > xstrdup(const char *str) > { > - size_t len; > char *cp; > > - len = strlen(str) + 1; > - cp = xmalloc(len); > - strlcpy(cp, str, len); > + if ((cp = strdup(str)) == NULL) > + fatal("xstrdup: could not allocate memory"); > return cp; > } > > @@ -96,17 +90,20 @@ xasprintf(char **ret, const char *fmt, . > } > > int > -xsnprintf(char *str, size_t size, const char *fmt, ...) > +xsnprintf(char *str, size_t len, const char *fmt, ...) > { > va_list ap; > int i; > > + if (len > INT_MAX) > + fatal("xsnprintf: len > INT_MAX"); > + > va_start(ap, fmt); > - i = vsnprintf(str, size, fmt, ap); > + i = vsnprintf(str, len, fmt, ap); > va_end(ap); > > - if (i == -1 || i >= (int)size) > - fatal("xsnprintf: overflow"); > + if (i < 0 || i >= (int)len) > + fatal("xsnprintf: could not allocate memory"); > > return (i); > } > Index: usr.bin/diff/xmalloc.c > =================================================================== > RCS file: /cvs/src/usr.bin/diff/xmalloc.c,v > retrieving revision 1.8 > diff -u -p -u -p -r1.8 xmalloc.c > --- usr.bin/diff/xmalloc.c 25 Sep 2015 16:16:26 -0000 1.8 > +++ usr.bin/diff/xmalloc.c 5 Nov 2015 14:42:09 -0000 > @@ -27,9 +27,11 @@ xmalloc(size_t size) > { > void *ptr; > > + if (size == 0) > + errx(2, "xmalloc: zero size"); > ptr = malloc(size); > if (ptr == NULL) > - err(2, "xmalloc %zu", size); > + errx(2, "xmalloc: out of memory (allocating %zu bytes)", size); > return ptr; > } > > @@ -40,7 +42,7 @@ xcalloc(size_t nmemb, size_t size) > > ptr = calloc(nmemb, size); > if (ptr == NULL) > - err(2, "xcalloc: out of memory (allocating %zu*%zu bytes)", > + errx(2, "xcalloc: out of memory (allocating %zu * %zu bytes)", > nmemb, size); > return ptr; > } > @@ -52,7 +54,8 @@ xreallocarray(void *ptr, size_t nmemb, s > > new_ptr = reallocarray(ptr, nmemb, size); > if (new_ptr == NULL) > - err(2, "xrealloc %zu*%zu", nmemb, size); > + errx(2, "xreallocarray: out of memory " > + "(allocating %zu * %zu bytes)", nmemb, size); > return new_ptr; > } > > @@ -62,7 +65,7 @@ xstrdup(const char *str) > char *cp; > > if ((cp = strdup(str)) == NULL) > - err(1, "xstrdup"); > + errx(2, "xstrdup: could not allocate memory"); > return cp; > } > > @@ -77,7 +80,7 @@ xasprintf(char **ret, const char *fmt, . > va_end(ap); > > if (i < 0 || *ret == NULL) > - err(2, "xasprintf"); > + errx(2, "xasprintf: could not allocate memory"); > > return (i); > } > Index: usr.bin/file/xmalloc.c > =================================================================== > RCS file: /cvs/src/usr.bin/file/xmalloc.c,v > retrieving revision 1.2 > diff -u -p -u -p -r1.2 xmalloc.c > --- usr.bin/file/xmalloc.c 17 Jun 2015 18:51:11 -0000 1.2 > +++ usr.bin/file/xmalloc.c 5 Nov 2015 14:42:09 -0000 > @@ -31,9 +31,7 @@ xmalloc(size_t size) > errx(1, "xmalloc: zero size"); > ptr = malloc(size); > if (ptr == NULL) > - errx(1, > - "xmalloc: out of memory (allocating %zu bytes)", > - size); > + errx(1, "xmalloc: out of memory (allocating %zu bytes)", size); > return ptr; > } > > @@ -44,12 +42,10 @@ xcalloc(size_t nmemb, size_t size) > > if (size == 0 || nmemb == 0) > errx(1, "xcalloc: zero size"); > - if (SIZE_MAX / nmemb < size) > - errx(1, "xcalloc: nmemb * size > SIZE_MAX"); > ptr = calloc(nmemb, size); > if (ptr == NULL) > - errx(1, "xcalloc: out of memory (allocating %zu bytes)", > - (size * nmemb)); > + errx(1, "xcalloc: out of memory (allocating %zu * %zu bytes)", > + nmemb, size); > return ptr; > } > > @@ -60,8 +56,8 @@ xreallocarray(void *ptr, size_t nmemb, s > > new_ptr = reallocarray(ptr, nmemb, size); > if (new_ptr == NULL) > - errx(1, "xreallocarray: out of memory (new_size %zu bytes)", > - nmemb * size); > + errx(1, "xreallocarray: out of memory " > + "(allocating %zu * %zu bytes)", nmemb, size); > return new_ptr; > } > > @@ -71,7 +67,7 @@ xstrdup(const char *str) > char *cp; > > if ((cp = strdup(str)) == NULL) > - err(1, "xstrdup"); > + errx(1, "xstrdup: could not allocate memory"); > return cp; > } > > Index: usr.bin/rcs/xmalloc.c > =================================================================== > RCS file: /cvs/src/usr.bin/rcs/xmalloc.c,v > retrieving revision 1.10 > diff -u -p -u -p -r1.10 xmalloc.c > --- usr.bin/rcs/xmalloc.c 17 Jun 2015 20:50:10 -0000 1.10 > +++ usr.bin/rcs/xmalloc.c 5 Nov 2015 14:42:13 -0000 > @@ -31,9 +31,7 @@ xmalloc(size_t size) > errx(1, "xmalloc: zero size"); > ptr = malloc(size); > if (ptr == NULL) > - errx(1, > - "xmalloc: out of memory (allocating %zu bytes)", > - size); > + errx(1, "xmalloc: out of memory (allocating %zu bytes)", size); > return ptr; > } > > @@ -44,12 +42,10 @@ xcalloc(size_t nmemb, size_t size) > > if (size == 0 || nmemb == 0) > errx(1, "xcalloc: zero size"); > - if (SIZE_MAX / nmemb < size) > - errx(1, "xcalloc: nmemb * size > SIZE_MAX"); > ptr = calloc(nmemb, size); > if (ptr == NULL) > - errx(1, "xcalloc: out of memory (allocating %zu bytes)", > - (size * nmemb)); > + errx(1, "xcalloc: out of memory (allocating %zu * %zu bytes)", > + nmemb, size); > return ptr; > } > > @@ -60,8 +56,8 @@ xreallocarray(void *ptr, size_t nmemb, s > > new_ptr = reallocarray(ptr, nmemb, size); > if (new_ptr == NULL) > - errx(1, "xreallocarray: out of memory (new_size %zu bytes)", > - nmemb * size); > + errx(1, "xreallocarray: out of memory " > + "(allocating %zu * %zu bytes)", nmemb, size); > return new_ptr; > } > > @@ -71,7 +67,7 @@ xstrdup(const char *str) > char *cp; > > if ((cp = strdup(str)) == NULL) > - err(1, "xstrdup"); > + errx(1, "xstrdup: could not allocate memory"); > return cp; > } > > Index: usr.bin/ssh/xmalloc.c > =================================================================== > RCS file: /cvs/src/usr.bin/ssh/xmalloc.c,v > retrieving revision 1.32 > diff -u -p -u -p -r1.32 xmalloc.c > --- usr.bin/ssh/xmalloc.c 24 Apr 2015 01:36:01 -0000 1.32 > +++ usr.bin/ssh/xmalloc.c 5 Nov 2015 14:42:17 -0000 > @@ -42,12 +42,10 @@ xcalloc(size_t nmemb, size_t size) > > if (size == 0 || nmemb == 0) > fatal("xcalloc: zero size"); > - if (SIZE_MAX / nmemb < size) > - fatal("xcalloc: nmemb * size > SIZE_MAX"); > ptr = calloc(nmemb, size); > if (ptr == NULL) > - fatal("xcalloc: out of memory (allocating %zu bytes)", > - size * nmemb); > + fatal("xcalloc: out of memory (allocating %zu * %zu bytes)", > + nmemb, size); > return ptr; > } > > @@ -58,20 +56,18 @@ xreallocarray(void *ptr, size_t nmemb, s > > new_ptr = reallocarray(ptr, nmemb, size); > if (new_ptr == NULL) > - fatal("xreallocarray: out of memory (%zu elements of %zu > bytes)", > - nmemb, size); > + fatal("xreallocarray: out of memory " > + "(allocating %zu * %zu bytes)", nmemb, size); > return new_ptr; > } > > char * > xstrdup(const char *str) > { > - size_t len; > char *cp; > > - len = strlen(str) + 1; > - cp = xmalloc(len); > - strlcpy(cp, str, len); > + if ((cp = strdup(str)) == NULL) > + fatal("xstrdup: could not allocate memory"); > return cp; > } > > Index: usr.bin/tmux/xmalloc.c > =================================================================== > RCS file: /cvs/src/usr.bin/tmux/xmalloc.c,v > retrieving revision 1.7 > diff -u -p -u -p -r1.7 xmalloc.c > --- usr.bin/tmux/xmalloc.c 20 Oct 2014 23:57:14 -0000 1.7 > +++ usr.bin/tmux/xmalloc.c 5 Nov 2015 14:42:18 -0000 > @@ -25,16 +25,13 @@ > #include "tmux.h" > > char * > -xstrdup(const char *s) > +xstrdup(const char *str) > { > - char *ptr; > - size_t len; > + char *cp; > > - len = strlen(s) + 1; > - ptr = xmalloc(len); > - > - strlcpy(ptr, s, len); > - return (ptr); > + if ((cp = strdup(str)) == NULL) > + fatalx("xstrdup: could not allocate memory"); > + return cp; > } > > void * > @@ -43,11 +40,10 @@ xcalloc(size_t nmemb, size_t size) > void *ptr; > > if (size == 0 || nmemb == 0) > - fatalx("zero size"); > - if (SIZE_MAX / nmemb < size) > - fatalx("nmemb * size > SIZE_MAX"); > + fatalx("xcalloc: zero size"); > if ((ptr = calloc(nmemb, size)) == NULL) > - fatal("xcalloc failed"); > + log_fatalx("xcalloc: out of memory (allocating %zu bytes)", > + size); > > return (ptr); > } > @@ -58,9 +54,10 @@ xmalloc(size_t size) > void *ptr; > > if (size == 0) > - fatalx("zero size"); > + fatalx("xmalloc: zero size"); > if ((ptr = malloc(size)) == NULL) > - fatal("xmalloc failed"); > + log_fatalx("xmalloc: out of memory (allocating %zu bytes)", > + size); > > return (ptr); > } > @@ -71,9 +68,10 @@ xrealloc(void *oldptr, size_t newsize) > void *newptr; > > if (newsize == 0) > - fatalx("zero size"); > + fatalx("xrealloc: zero size"); > if ((newptr = realloc(oldptr, newsize)) == NULL) > - fatal("xrealloc failed"); > + log_fatalx("xrealloc: out of memory (allocating %zu bytes)", > + newsize); > > return (newptr); > } > @@ -81,15 +79,13 @@ xrealloc(void *oldptr, size_t newsize) > void * > xreallocarray(void *oldptr, size_t nmemb, size_t size) > { > - size_t newsize = nmemb * size; > void *newptr; > > - if (newsize == 0) > - fatalx("zero size"); > - if (SIZE_MAX / nmemb < size) > - fatalx("nmemb * size > SIZE_MAX"); > - if ((newptr = realloc(oldptr, newsize)) == NULL) > - fatal("xreallocarray failed"); > + if (nmemb == 0 || size == 0) > + fatalx("xreallocarray: zero size"); > + if ((newptr = reallocarray(oldptr, nmemb, size)) == NULL) > + log_fatalx("xreallocarray: out of memory " > + "(allocating %zu * %zu bytes)", nmemb, size); > > return (newptr); > } > @@ -114,7 +110,7 @@ xvasprintf(char **ret, const char *fmt, > > i = vasprintf(ret, fmt, ap); > if (i < 0 || *ret == NULL) > - fatal("xvasprintf failed"); > + fatalx("xvasprintf: could not allocate memory"); > > return (i); > } > @@ -138,11 +134,11 @@ xvsnprintf(char *buf, size_t len, const > int i; > > if (len > INT_MAX) > - fatalx("len > INT_MAX"); > + fatalx("xvsnprintf: len > INT_MAX"); > > i = vsnprintf(buf, len, fmt, ap); > - if (i < 0) > - fatal("vsnprintf failed"); > + if (i < 0 || i >= (int)len) > + fatalx("xvsnprintf: could not allocate memory"); > > return (i); > }