diff mbox

Do we care about malloc failure in tests?

Message ID CALoOobP0AHXDbX6qiKtpXOhwH8GY+MDVwaxfTSWakumW7+HN4Q@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov June 6, 2016, 12:39 a.m. UTC 
On Tue, May 31, 2016 at 3:02 PM, Mike Frysinger <vapier@gentoo.org> wrote:

> i'd really like to see xmalloc type helpers added to the test skeleton
> rather than change every test to include the same boiler plate

How does attached patch look?

If it's ok, I can commit it and then convert malloc/calloc/realloc in
tests to xmalloc and friends.

Thanks,

2016-06-05  Paul Pluzhnikov  <ppluzhnikov@google.com>

        * test-skeleton.c (oom_error, xmalloc, xcalloc, xrealloc): New
functions.
        (add_temp_file): Use them.

Comments

Mike Frysinger June 6, 2016, 1:54 a.m. UTC | #1 
On 05 Jun 2016 17:39, Paul Pluzhnikov wrote:
> +static void
> +oom_error (const char *fn)
> +{
> +  fprintf (stderr, "Out of memory in %s\n", fn);

use printf as we don't let test write to stderr anymore

might want to tweak the printf like so:
  printf ("%s: allocation failed: %m\n", fn);
-mike
Paul Pluzhnikov June 6, 2016, 2 a.m. UTC | #2 
On Sun, Jun 5, 2016 at 6:54 PM, Mike Frysinger <vapier@gentoo.org> wrote:

> use printf as we don't let test write to stderr anymore
>
> might want to tweak the printf like so:
>   printf ("%s: allocation failed: %m\n", fn);

Done.

I think I can also remove this from xrealloc, since our realloc does
the right thing when p==NULL.

+  if (p == 0)
+    return xmalloc (n);

Ok to commit with these changes?

Thanks,
Mike Frysinger June 6, 2016, 2:12 a.m. UTC | #3 
On 05 Jun 2016 19:00, Paul Pluzhnikov wrote:
> I think I can also remove this from xrealloc, since our realloc does
> the right thing when p==NULL.
> 
> +  if (p == 0)
> +    return xmalloc (n);

right, we don't want/need that

i think all the use of 0 should be changed to NULL too
-mike
Florian Weimer June 7, 2016, 4:44 p.m. UTC | #4 
On 06/06/2016 03:54 AM, Mike Frysinger wrote:
> On 05 Jun 2016 17:39, Paul Pluzhnikov wrote:
>> +static void
>> +oom_error (const char *fn)
>> +{
>> +  fprintf (stderr, "Out of memory in %s\n", fn);
>
> use printf as we don't let test write to stderr anymore
>
> might want to tweak the printf like so:
>   printf ("%s: allocation failed: %m\n", fn);

Perhaps also include the request allocation size as well.

Thanks,
Florian
diff mbox

Patch

diff --git a/test-skeleton.c b/test-skeleton.c
index 29bdc9c..1be312d 100644
--- a/test-skeleton.c
+++ b/test-skeleton.c
@@ -70,6 +70,54 @@  static pid_t pid;
 /* Directory to place temporary files in.  */
 static const char *test_dir;
 
+static void
+oom_error (const char *fn)
+{
+  fprintf (stderr, "Out of memory in %s\n", fn);
+  exit (1);
+}
+
+/* Allocate N bytes of memory dynamically, with error checking.  */
+static void *
+__attribute__ ((used))
+xmalloc (size_t n)
+{
+  void *p;
+
+  p = malloc (n);
+  if (p == 0)
+    oom_error ("malloc");
+  return p;
+}
+
+/* Allocate memory for N elements of S bytes, with error checking.  */
+static void *
+__attribute__ ((used))
+xcalloc (size_t n, size_t s)
+{
+  void *p;
+
+  p = calloc (n, s);
+  if (p == 0)
+    oom_error ("calloc");
+  return p;
+}
+
+/* Change the size of an allocated block of memory P to N bytes,
+   with error checking.
+   If P is NULL, run xmalloc.  */
+static void *
+__attribute__ ((used))
+xrealloc (void *p, size_t n)
+{
+  if (p == 0)
+    return xmalloc (n);
+  p = realloc (p, n);
+  if (p == 0)
+    oom_error ("realloc");
+  return p;
+}
+
 /* List of temporary files.  */
 struct temp_name_list
 {
@@ -83,9 +131,9 @@  __attribute__ ((unused))
 add_temp_file (const char *name)
 {
   struct temp_name_list *newp
-    = (struct temp_name_list *) calloc (sizeof (*newp), 1);
+    = (struct temp_name_list *) xcalloc (sizeof (*newp), 1);
   char *newname = strdup (name);
-  if (newp != NULL && newname != NULL)
+  if (newname != NULL)
     {
       newp->name = newname;
       if (temp_name_list == NULL)
@@ -124,13 +172,8 @@  create_temp_file (const char *base, char **filename)
   char *fname;
   int fd;
 
-  fname = (char *) malloc (strlen (test_dir) + 1 + strlen (base)
-			   + sizeof ("XXXXXX"));
-  if (fname == NULL)
-    {
-      puts ("out of memory");
-      return -1;
-    }
+  fname = (char *) xmalloc (strlen (test_dir) + 1 + strlen (base)
+			    + sizeof ("XXXXXX"));
   strcpy (stpcpy (stpcpy (stpcpy (fname, test_dir), "/"), base), "XXXXXX");
 
   fd = mkstemp (fname);