Hi Bruno,
On Thu, 2009-02-19 at 23:59 +0100, Bruno Haible wrote:
> Thanks for proposing such a nice module!
All glory for this lies with Dan (and Jim Meyering for doing a
preliminary review)
> For a module as basic as this one, in gnulib, I think it's essential that the
> copyright is with the FSF. Can you arrange with Daniel Berrange and/or his
> employer (Red Hat) that the copyright gets assigned?
I was told that Red Hat has a blanket agreement with the FSF for
copyright assignment. Whatever I did for this module, I did during work
hours, so copyright belongs to Red Hat. I think that is also true for
Dan (cc'd him so he can confirm that explicitly)
Can somebody from the FSF confirm that ? I did adjust the copyright
notice in the source files.
> Otherwise, looks well done. I see only a couple of minor points to be
> resolved:
> - avoid collision between xalloc.h and safe-alloc.h over xalloc_oversized,
Ok. I'll rename xalloc_oversized to safe_alloc_oversized
> - use xalloc_oversized also before calling calloc, because most libc
> implementations will not have the overflow check,
Ok.
> - use GNU conventions (space between identifier and opening parenthesis,
> placement of braces) in safe-alloc.h,
Oops .. 'indent --no-tabs' doesn't do macros, of course.
> - perhaps a unit test?
Added a very basic one.
Attached is an incremental patch with those changes.
David
>From 1480d4e7caaa85ce7d572f84aec2f6048f014f75 Mon Sep 17 00:00:00 2001
From: David Lutterkort <lut...@redhat.com>
Date: Thu, 19 Feb 2009 16:06:49 -0800
Subject: [PATCH] Changes suggested by Bruno Haible
* rename xalloc_oversized to safe_alloc_oversized to avoid collision with
xalloc.h
* use safe_alloc_oversized before call to calloc, too
* fix indentation to be (hopefully) in accordance with GNU standards
* adjust copyright notice for (C) FSF
* add a simple unit test
---
lib/safe-alloc.c | 32 ++++++++++++-----------
lib/safe-alloc.h | 49 +++++++++++++++++++----------------
modules/safe-alloc-tests | 10 +++++++
tests/test-safe-alloc.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 116 insertions(+), 38 deletions(-)
create mode 100644 modules/safe-alloc-tests
create mode 100644 tests/test-safe-alloc.c
diff --git a/lib/safe-alloc.c b/lib/safe-alloc.c
index 640eeb7..fca8754 100644
--- a/lib/safe-alloc.c
+++ b/lib/safe-alloc.c
@@ -1,7 +1,7 @@
/*
* safe-alloc.c: safer memory allocation
*
- * Copyright (C) 2008, 2009 Daniel P. Berrange
+ * Copyright (C) 2009 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -19,6 +19,8 @@
*
*/
+/* Written by Daniel Berrange <berra...@redhat.com>, 2008 */
+
#include <config.h>
#include <stdlib.h>
@@ -39,9 +41,12 @@
However, malloc (SIZE_MAX) fails on all known hosts where
sizeof (ptrdiff_t) <= sizeof (size_t), so do not bother to test for
exactly-SIZE_MAX allocations on such hosts; this avoids a test and
- branch when S is known to be 1. */
-# define xalloc_oversized(n, s) \
- ((size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) < (n))
+ branch when S is known to be 1.
+
+ This is the same as xalloc_oversized from xalloc.h
+*/
+#define safe_alloc_oversized(n, s) \
+ ((size_t) (sizeof (ptrdiff_t) <= sizeof (size_t) ? -1 : -2) / (s) < (n))
/**
@@ -66,19 +71,16 @@ safe_alloc_alloc_n (void *ptrptr, size_t size, size_t count, int zeroed)
return 0;
}
- if (zeroed)
+ if (safe_alloc_oversized (count, size))
{
- *(void **) ptrptr = calloc (count, size);
+ errno = ENOMEM;
+ return -1;
}
+
+ if (zeroed)
+ *(void **) ptrptr = calloc (count, size);
else
- {
- if (xalloc_oversized (count, size))
- {
- errno = ENOMEM;
- return -1;
- }
- *(void **) ptrptr = malloc (count * size);
- }
+ *(void **) ptrptr = malloc (count * size);
if (*(void **) ptrptr == NULL)
return -1;
@@ -109,7 +111,7 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
*(void **) ptrptr = NULL;
return 0;
}
- if (xalloc_oversized (count, size))
+ if (safe_alloc_oversized (count, size))
{
errno = ENOMEM;
return -1;
diff --git a/lib/safe-alloc.h b/lib/safe-alloc.h
index 6e42512..1b7847a 100644
--- a/lib/safe-alloc.h
+++ b/lib/safe-alloc.h
@@ -1,7 +1,7 @@
/*
* memory.c: safer memory allocation
*
- * Copyright (C) 2008, 2009 Daniel P. Berrange
+ * Copyright (C) 2009 Free Software Foundation, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -19,19 +19,20 @@
*
*/
+/* Written by Daniel Berrange <berra...@redhat.com>, 2008 */
#ifndef SAFE_ALLOC_H_
-#define SAFE_ALLOC_H_
+# define SAFE_ALLOC_H_
-#include <stdlib.h>
+# include <stdlib.h>
-#ifndef ATTRIBUTE_RETURN_CHECK
-#if __GNUC_PREREQ (3, 4)
-#define ATTRIBUTE_RETURN_CHECK __attribute__((__warn_unused_result__))
-#else
-#define ATTRIBUTE_RETURN_CHECK
-#endif
-#endif
+# ifndef ATTRIBUTE_RETURN_CHECK
+# if __GNUC_PREREQ (3, 4)
+# define ATTRIBUTE_RETURN_CHECK __attribute__((__warn_unused_result__))
+# else
+# define ATTRIBUTE_RETURN_CHECK
+# endif
+# endif
/* Don't call these directly - use the macros below */
int
@@ -52,8 +53,8 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
*
* Return -1 on failure to allocate, zero on success
*/
-#define ALLOC(ptr) \
- safe_alloc_alloc_n(&(ptr), sizeof(*(ptr)), 1, 1)
+# define ALLOC(ptr) \
+ safe_alloc_alloc_n (&(ptr), sizeof(*(ptr)), 1, 1)
/**
* ALLOC_N:
@@ -66,8 +67,8 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
*
* 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_alloc_n (&(ptr), sizeof(*(ptr)), (count), 1)
/**
* ALLOC_N_UNINITIALIZED:
@@ -80,8 +81,8 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
*
* 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_alloc_n (&(ptr), sizeof(*(ptr)), (count), 0)
/**
* REALLOC_N:
@@ -94,8 +95,8 @@ safe_alloc_realloc_n (void *ptrptr, size_t size, size_t count)
*
* 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_realloc_n (&(ptr), sizeof(*(ptr)), (count))
/**
* FREE:
@@ -104,10 +105,12 @@ 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) \
+ do \
+ { \
+ free(ptr); \
+ (ptr) = NULL; \
+ } \
+ while(0)
#endif /* SAFE_ALLOC_H_ */
diff --git a/modules/safe-alloc-tests b/modules/safe-alloc-tests
new file mode 100644
index 0000000..dbd9c8f
--- /dev/null
+++ b/modules/safe-alloc-tests
@@ -0,0 +1,10 @@
+Files:
+tests/test-safe-alloc.c
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-safe-alloc
+check_PROGRAMS += test-safe-alloc
diff --git a/tests/test-safe-alloc.c b/tests/test-safe-alloc.c
new file mode 100644
index 0000000..420b5e2
--- /dev/null
+++ b/tests/test-safe-alloc.c
@@ -0,0 +1,63 @@
+/*
+ * Test the safe-alloc macros
+ *
+ * Copyright (C) 2009 Free Software Foundation, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: David Lutterkort <lut...@redhat.com>
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+
+#include <safe-alloc.h>
+
+#define ASSERT(expr) \
+ do \
+ { \
+ if (!(expr)) \
+ { \
+ fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \
+ fflush (stderr); \
+ abort (); \
+ } \
+ } \
+ while (0)
+
+int main()
+{
+ struct tst {
+ int a;
+ int b;
+ };
+
+ struct tst *p = NULL;
+ int r;
+
+ r = ALLOC(p);
+ ASSERT(r >= 0);
+
+ ASSERT(p->a == 0 && p->b == 0);
+
+ p->a = p->b = 42;
+ r = REALLOC_N(p, 5);
+
+ ASSERT(p[0].a == 42 && p[0].b == 42);
+
+ FREE(p);
+ ASSERT(p == NULL);
+}
--
1.6.0.6
