The old implementation assumed that all pointers use the same
internal representation, but the C standard doesn’t guarantee
this. Use void * (pointer) not void ** (pointer-to-pointer) for
the internal functions’ API. The internal functions now return
NULL if and only if they failed, and the macros translate that
into -1 or 0 to satisfy the existing API.
* doc/safe-alloc.texi (Safe Allocation Macros): Mention overflow.
* lib/safe-alloc.c: Major rewrite. Now this simply
defines SAFE_ALLOC_INLINE and includes safe-alloc.h.
* lib/safe-alloc.h: Include stddef.h, not stdlib.h.
(SAFE_ALLOC_INLINE): New macro; use Gnulib inline function style.
(safe_alloc_realloc_n): New API, which passes and returns
the pointer, and which returns NULL if and only if failure occurs.
(safe_alloc_check): New function.
(ALLOC, ALLOC_N, ALLOC_N_UNINITIALIZED, REALLOC_N):
Redo using the new API for internal functions, and using calloc
which is good enough since it’s GNU-compatible now.
(FREE): Expand to an expression rather than merely to something
that needs a following ‘;’ to become a statement.
* modules/safe-alloc (Depends-on): Add calloc-gnu.
---
ChangeLog | 22 +++++++++++
doc/safe-alloc.texi | 2 +
lib/safe-alloc.c | 90 +-------------------------------------------
lib/safe-alloc.h | 91 +++++++++++++++++++++++++--------------------
modules/safe-alloc | 1 +
5 files changed, 76 insertions(+), 130 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ede998670..2c7edd59a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,27 @@
2021-04-18 Paul Eggert <egg...@cs.ucla.edu>
+ safe-alloc: fix pointer implementation
+ The old implementation assumed that all pointers use the same
+ internal representation, but the C standard doesn’t guarantee
+ this. Use void * (pointer) not void ** (pointer-to-pointer) for
+ the internal functions’ API. The internal functions now return
+ NULL if and only if they failed, and the macros translate that
+ into -1 or 0 to satisfy the existing API.
+ * doc/safe-alloc.texi (Safe Allocation Macros): Mention overflow.
+ * lib/safe-alloc.c: Major rewrite. Now this simply
+ defines SAFE_ALLOC_INLINE and includes safe-alloc.h.
+ * lib/safe-alloc.h: Include stddef.h, not stdlib.h.
+ (SAFE_ALLOC_INLINE): New macro; use Gnulib inline function style.
+ (safe_alloc_realloc_n): New API, which passes and returns
+ the pointer, and which returns NULL if and only if failure occurs.
+ (safe_alloc_check): New function.
+ (ALLOC, ALLOC_N, ALLOC_N_UNINITIALIZED, REALLOC_N):
+ Redo using the new API for internal functions, and using calloc
+ which is good enough since it’s GNU-compatible now.
+ (FREE): Expand to an expression rather than merely to something
+ that needs a following ‘;’ to become a statement.
+ * modules/safe-alloc (Depends-on): Add calloc-gnu.
+
calloc-gnu: now LGPLv2+
* modules/calloc-gnu (License): Change from GPL to LGPLv2+.
The old value was evidently a longstanding typo, and calloc
diff --git a/doc/safe-alloc.texi b/doc/safe-alloc.texi
index d40ec65b6..e896e2598 100644
--- a/doc/safe-alloc.texi
+++ b/doc/safe-alloc.texi
@@ -13,6 +13,8 @@ Some of the memory allocation mistakes that are commonly made
are
passing the incorrect number of bytes to @code{malloc}, especially
when allocating an array,
@item
+unchecked integer overflow when calculating array sizes,
+@item
fail to check the return value of @code{malloc} and @code{realloc} for
errors,
@item
diff --git a/lib/safe-alloc.c b/lib/safe-alloc.c
index 116ac4371..df061f9ef 100644
--- a/lib/safe-alloc.c
+++ b/lib/safe-alloc.c
@@ -1,91 +1,3 @@
-/* safe-alloc.c: safer memory allocation
-
- Copyright (C) 2009-2021 Free Software Foundation, Inc.
-
- This program is free software: you can redistribute it and/or modify it
- under the terms of the GNU General Public License as published by the
- Free Software Foundation; either version 3 of the License, or any
- later version.
-
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
-
- You should have received a copy of the GNU General Public License
- along with this program. If not, see <https://www.gnu.org/licenses/>. */
-
-/* Written by Daniel Berrange <berra...@redhat.com>, 2008 */
-
#include <config.h>
-
-/* Specification. */
+#define SAFE_ALLOC_INLINE _GL_EXTERN_INLINE
#include "safe-alloc.h"
-
-#include <stdlib.h>
-#include <stddef.h>
-#include <errno.h>
-
-
-/**
- * safe_alloc_alloc_n:
- * @ptrptr: pointer to pointer for address of allocated memory
- * @size: number of bytes to allocate
- * @count: number of elements to allocate
- *
- * Allocate an array of memory 'count' elements long,
- * each with 'size' bytes. Return the address of the
- * allocated memory in 'ptrptr'. The newly allocated
- * memory is filled with zeros.
- *
- * Return -1 on failure to allocate, zero on success
- */
-int
-safe_alloc_alloc_n (void *ptrptr, size_t size, size_t count, int zeroed)
-{
- if (size == 0 || count == 0)
- {
- *(void **) ptrptr = NULL;
- return 0;
- }
-
- if (zeroed)
- *(void **) ptrptr = calloc (count, size);
- else
- *(void **) ptrptr = reallocarray (NULL, count, size);
-
- if (*(void **) ptrptr == NULL)
- return -1;
- return 0;
-}
-
-/**
- * safe_alloc_realloc_n:
- * @ptrptr: pointer to pointer for address of allocated memory
- * @size: number of bytes to allocate
- * @count: number of elements in array
- *
- * Resize the block of memory in 'ptrptr' to be an array of
- * 'count' elements, each 'size' bytes in length. Update 'ptrptr'
- * with the address of the newly allocated memory. On failure,
- * 'ptrptr' is not changed and still points to the original memory
- * block. The newly allocated memory is filled with zeros.
- *
- * Return -1 on failure to allocate, zero on success
- */
-int
-safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
-{
- void *tmp;
- if (size == 0 || count == 0)
- {
- free (*(void **) ptrptr);
- *(void **) ptrptr = NULL;
- return 0;
- }
- tmp = reallocarray (*(void **) ptrptr, size, count);
- if (!tmp)
- return -1;
- *(void **) ptrptr = tmp;
- return 0;
-}
diff --git a/lib/safe-alloc.h b/lib/safe-alloc.h
index 1c655734e..7de424f3c 100644
--- a/lib/safe-alloc.h
+++ b/lib/safe-alloc.h
@@ -15,76 +15,89 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>. */
-/* Written by Daniel Berrange <berra...@redhat.com>, 2008 */
+/* Written by Daniel Berrange and Paul Eggert. */
#ifndef SAFE_ALLOC_H_
-# define SAFE_ALLOC_H_
+#define SAFE_ALLOC_H_
-# include <stdlib.h>
+#include <stdlib.h>
-/* Don't call these directly - use the macros below */
-int
-safe_alloc_alloc_n (void *ptrptr, size_t size, size_t count, int zeroed)
- _GL_ATTRIBUTE_NODISCARD;
+#ifndef _GL_INLINE_HEADER_BEGIN
+ #error "Please include config.h first."
+#endif
+_GL_INLINE_HEADER_BEGIN
+#ifndef SAFE_ALLOC_INLINE
+# define SAFE_ALLOC_INLINE _GL_INLINE
+#endif
-int
-safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
- _GL_ATTRIBUTE_NODISCARD;
+/* Don't call these directly - use the macros below. */
+SAFE_ALLOC_INLINE void *
+safe_alloc_realloc_n (void *ptr, size_t count, size_t size)
+{
+ if (count == 0 || size == 0)
+ count = size = 1;
+ return reallocarray (ptr, count, size);
+}
+SAFE_ALLOC_INLINE int _GL_ATTRIBUTE_NODISCARD
+safe_alloc_check (void *ptr)
+{
+ /* Return 0 if the allocation was successful, -1 otherwise. */
+ return -!ptr;
+}
/**
* ALLOC:
- * @ptr: pointer to hold address of allocated memory
+ * @ptr: pointer to allocated memory
*
- * Allocate sizeof(*ptr) bytes of memory and store
- * the address of allocated memory in 'ptr'. Fill the
+ * Allocate sizeof *ptr bytes of memory and store
+ * the address of allocated memory in 'ptr'. Fill the
* newly allocated memory with zeros.
*
- * Return -1 on failure to allocate, zero on success
+ * Return -1 on failure to allocate, zero on success.
*/
-# define ALLOC(ptr) \
- safe_alloc_alloc_n (&(ptr), sizeof (*(ptr)), 1, 1)
+#define ALLOC(ptr) ALLOC_N (ptr, 1)
/**
* ALLOC_N:
- * @ptr: pointer to hold address of allocated memory
+ * @ptr: pointer to allocated memory
* @count: number of elements to allocate
*
- * Allocate an array of 'count' elements, each sizeof(*ptr)
+ * Allocate an array of 'count' elements, each sizeof *ptr
* bytes long and store the address of allocated memory in
- * 'ptr'. Fill the newly allocated memory with zeros.
+ * 'ptr'. Fill the newly allocated memory with zeros.
*
- * Return -1 on failure, 0 on success
+ * Return -1 on failure, 0 on success.
*/
-# define ALLOC_N(ptr, count) \
- safe_alloc_alloc_n (&(ptr), sizeof (*(ptr)), (count), 1)
+#define ALLOC_N(ptr, count) \
+ safe_alloc_check ((ptr) = calloc (count, sizeof *(ptr)))
/**
* ALLOC_N_UNINITIALIZED:
- * @ptr: pointer to hold address of allocated memory
+ * @ptr: pointer to allocated memory
* @count: number of elements to allocate
*
- * Allocate an array of 'count' elements, each sizeof(*ptr)
+ * Allocate an array of 'count' elements, each sizeof *ptr
* bytes long and store the address of allocated memory in
- * 'ptr'. Do not initialize the new memory at all.
+ * 'ptr'. Do not initialize the new memory at all.
*
- * Return -1 on failure to allocate, zero on success
+ * Return -1 on failure to allocate, zero on success.
*/
-# define ALLOC_N_UNINITIALIZED(ptr, count) \
- safe_alloc_alloc_n (&(ptr), sizeof (*(ptr)), (count), 0)
+#define ALLOC_N_UNINITIALIZED(ptr, count) \
+ safe_alloc_check ((ptr) = safe_alloc_realloc_n (NULL, count, sizeof *(ptr)))
/**
* REALLOC_N:
- * @ptr: pointer to hold address of allocated memory
+ * @ptr: pointer to allocated memory
* @count: number of elements to allocate
*
- * Re-allocate an array of 'count' elements, each sizeof(*ptr)
+ * Re-allocate an array of 'count' elements, each sizeof *ptr
* bytes long and store the address of allocated memory in
- * 'ptr'. Fill the newly allocated memory with zeros
+ * 'ptr'. Fill the newly allocated memory with zeros.
*
- * Return -1 on failure to reallocate, zero on success
+ * Return -1 on failure to reallocate, zero on success.
*/
-# define REALLOC_N(ptr, count) \
- safe_alloc_realloc_n (&(ptr), sizeof (*(ptr)), (count))
+#define REALLOC_N(ptr, count) \
+ safe_alloc_check ((ptr) = safe_alloc_realloc_n (ptr, count, sizeof *(ptr)))
/**
* FREE:
@@ -93,12 +106,8 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t
count)
* Free the memory stored in 'ptr' and update to point
* to NULL.
*/
-# define FREE(ptr) \
- do \
- { \
- free (ptr); \
- (ptr) = NULL; \
- } \
- while (0)
+#define FREE(ptr) ((void) (free (ptr), (ptr) = NULL))
+
+_GL_INLINE_HEADER_END
#endif /* SAFE_ALLOC_H_ */
diff --git a/modules/safe-alloc b/modules/safe-alloc
index 9453b49ee..180aa8a2d 100644
--- a/modules/safe-alloc
+++ b/modules/safe-alloc
@@ -7,6 +7,7 @@ lib/safe-alloc.c
m4/safe-alloc.m4
Depends-on:
+calloc-gnu
reallocarray
configure.ac:
--
2.27.0
