two new leaks [Re: 10 more change sets for review

Jim Meyering jim at meyering.net
Thu Oct 21 12:21:43 UTC 2010


Jim Meyering wrote:

> Jeff Darcy wrote:
>> On 10/20/2010 04:28 PM, Jim Meyering wrote:
>>> [an hour or two later...]
>>> I think it's done.  A little more testing and I'll post the patch.
>>> That means tomorrow.
>>
>> It sounds like you're on track to something more robust.  Thank you for
>> implementing it.
>
> I've pushed that series, but without the c-set you questioned.
> I'll repost the replacement shortly.

Here's more clean-up and an error-path leak fix (not tested).
This applies on top of the "parser: handle OOM gracefully" c-set:

  [PATCH 1/4] parser: remove dead code
  [PATCH 2/4] parser: move x*alloc functions into #if-unit-test block where used
  [PATCH 3/4] plug error path leak
  [PATCH 4/4] reduce scope of global to be file-only

>From 6107f510a949b3efbebd001be2b369dac498d184 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 20 Oct 2010 21:34:08 +0200
Subject: [PATCH 1/4] parser: remove dead code

* qparser.y (xrealloc): Remove if-0'd function.
---
 qparser.y |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/qparser.y b/qparser.y
index 82b5125..cb6d36e 100644
--- a/qparser.y
+++ b/qparser.y
@@ -50,19 +50,6 @@ xmalloc (size_t n)
   return p;
 }

-#if 0 // not used
-/* Change the size of an allocated block of memory P to N bytes,
-   with error checking.  */
-static void *
-xrealloc (void *p, size_t n)
-{
-  p = realloc (p, n);
-  if (!p && n != 0)
-    xalloc_die ();
-  return p;
-}
-#endif
-
 /* Clone an object P of size S, with error checking.  There's no need
    for xnmemdup (P, N, S), since xmemdup (P, N * S) works without any
    need for an arithmetic overflow check.  */
--
1.7.3.1.216.g329be


>From 4de58f38c861643cad4393713e6127c9792d62a7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 20 Oct 2010 21:36:37 +0200
Subject: [PATCH 2/4] parser: move x*alloc functions into #if-unit-test block where used

* qparser.y (xalloc_die, xmalloc, xmemdup, xstrdup): Move functions...
[#if PARSER_UNIT_TEST]: ... into this #if-block, since now they're
used only there.
---
 qparser.y |   77 +++++++++++++++++++++++++++++++------------------------------
 1 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/qparser.y b/qparser.y
index cb6d36e..a23b5a7 100644
--- a/qparser.y
+++ b/qparser.y
@@ -28,44 +28,6 @@ static value_t invalid = { T_INVALID, {0}, NULL };
 #define YY_DECL int yylex(YYSTYPE *, void *scanner);
 YY_DECL

-static void
-xalloc_die (void)
-{
-  error (EXIT_FAILURE, 0, "%s", "memory exhausted");
-
-  /* The `noreturn' cannot be given to error, since it may return if
-     its first argument is 0.  To help compilers understand the
-     xalloc_die does not return, call abort.  Also, the abort is a
-     safety feature if exit_failure is 0 (which shouldn't happen).  */
-  abort ();
-}
-
-/* Allocate N bytes of memory dynamically, with error checking.  */
-static void *
-xmalloc (size_t n)
-{
-  void *p = malloc (n);
-  if (!p && n != 0)
-    xalloc_die ();
-  return p;
-}
-
-/* Clone an object P of size S, with error checking.  There's no need
-   for xnmemdup (P, N, S), since xmemdup (P, N * S) works without any
-   need for an arithmetic overflow check.  */
-static void *
-xmemdup (void const *p, size_t s)
-{
-  return memcpy (xmalloc (s), p, s);
-}
-
-/* Clone STRING.  */
-static char *
-xstrdup (char const *string)
-{
-  return xmemdup (string, strlen (string) + 1);
-}
-
 /* TBD: use separate function to parse dates differently */
 static value_t *
 make_number (const char *text)
@@ -292,6 +254,45 @@ paren_expr:
 %%

 #if defined PARSER_UNIT_TEST
+
+static void
+xalloc_die (void)
+{
+  error (EXIT_FAILURE, 0, "%s", "memory exhausted");
+
+  /* The `noreturn' cannot be given to error, since it may return if
+     its first argument is 0.  To help compilers understand the
+     xalloc_die does not return, call abort.  Also, the abort is a
+     safety feature if exit_failure is 0 (which shouldn't happen).  */
+  abort ();
+}
+
+/* Allocate N bytes of memory dynamically, with error checking.  */
+static void *
+xmalloc (size_t n)
+{
+  void *p = malloc (n);
+  if (!p && n != 0)
+    xalloc_die ();
+  return p;
+}
+
+/* Clone an object P of size S, with error checking.  There's no need
+   for xnmemdup (P, N, S), since xmemdup (P, N * S) works without any
+   need for an arithmetic overflow check.  */
+static void *
+xmemdup (void const *p, size_t s)
+{
+  return memcpy (xmalloc (s), p, s);
+}
+
+/* Clone STRING.  */
+static char *
+xstrdup (char const *string)
+{
+  return xmemdup (string, strlen (string) + 1);
+}
+
 static const struct { char *name; char *value; } hacked_obj_fields[] = {
         /* Fake object fields for generic unit testing. */
 	{ "a", "2" }, { "b", "7" }, { "c", "11" },
--
1.7.3.1.216.g329be


>From 45976a2e406500090b1909722f3614ce12f32575 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 21 Oct 2010 08:41:59 +0200
Subject: [PATCH 3/4] plug error path leak

---
 rest.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/rest.c b/rest.c
index 28bc401..cbd556c 100644
--- a/rest.c
+++ b/rest.c
@@ -905,6 +905,7 @@ proxy_delete (void *cctx, struct MHD_Connection *conn, const char *url,

 	resp = MHD_create_response_from_data(0,NULL,MHD_NO,MHD_NO);
 	if (!resp) {
+		free_ms(ms);
 		return MHD_NO;
 	}
 	MHD_queue_response(conn,rc,resp);
--
1.7.3.1.216.g329be


>From 1f6b543be591d996abe592e642a7a323bbd2a7f7 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Thu, 21 Oct 2010 09:08:39 +0200
Subject: [PATCH 4/4] reduce scope of global to be file-only

* qparser.y (hacked_links): Declare to be "static const".
---
 qparser.y |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qparser.y b/qparser.y
index a23b5a7..1c6530a 100644
--- a/qparser.y
+++ b/qparser.y
@@ -329,7 +329,7 @@ unit_sget_func (void * notused, const char *text)
 static const getter_t unit_sget = { unit_sget_func };

 /* Fake links from an object/key tuple to an object/key string. */
-struct { char *obj; char *key; char *value; } hacked_links[] = {
+static const struct { char *obj; char *key; char *value; } hacked_links[] = {
 	{ "templates/the_tmpl", "owner", "users/the_user" },
 	{ "users/the_user", "name", "Jeff Darcy" },
 	{ NULL }
--
1.7.3.1.216.g329be


More information about the iwhd-devel mailing list