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);
> }
