[Cluster-devel] Re: [PATCH cluster 2/5] dlm_controld: handle heap allocation failure and plug leaks

[Jim Meyering]

Jim Meyering wrote:

> From: Jim Meyering <meyering at redhat.com>
>
> * group/dlm_controld/pacemaker.c (process_cluster): Don't dereference
> NULL upon failing malloc or realloc.  Free "header" upon failure.
> ---
>  group/dlm_controld/pacemaker.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/group/dlm_controld/pacemaker.c b/group/dlm_controld/pacemaker.c
> index fed9ca7..b9b38d0 100644
> --- a/group/dlm_controld/pacemaker.c
> +++ b/group/dlm_controld/pacemaker.c
> @@ -135,10 +135,12 @@ void process_cluster(int ci)
>
>      AIS_Message *msg = NULL;
>      SaAisErrorT rc = SA_AIS_OK;
> -    mar_res_header_t *header = NULL;
> +    mar_res_header_t *header;
> +    mar_res_header_t *h;

Oops.  That "h" have been "h_new".
(Andrew Beekhof noticed this)
...
> +    h_new = realloc(header, header->size);

Here's the corrected patch:

>From bbc3ffcaf72d8089a1783af8ec9dc5b726a2b68e Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Mon, 22 Jun 2009 23:37:21 +0200
Subject: [PATCH cluster] dlm_controld: handle heap allocation failure and plug leaks

* group/dlm_controld/pacemaker.c (process_cluster): Don't dereference
NULL upon failing malloc or realloc.  Free "header" upon failure.
---
 group/dlm_controld/pacemaker.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/group/dlm_controld/pacemaker.c b/group/dlm_controld/pacemaker.c
index 509696a..9b0a2d8 100644
--- a/group/dlm_controld/pacemaker.c
+++ b/group/dlm_controld/pacemaker.c
@@ -135,10 +135,12 @@ void process_cluster(int ci)

     AIS_Message *msg = NULL;
     SaAisErrorT rc = SA_AIS_OK;
-    mar_res_header_t *header = NULL;
+    mar_res_header_t *header;
+    mar_res_header_t *h_new;
     static int header_len = sizeof(mar_res_header_t);

-    header = malloc(header_len);
+    if ((header = malloc(header_len)) == NULL)
+	goto bail;
     memset(header, 0, header_len);
     
     errno = 0;
@@ -160,8 +162,12 @@ void process_cluster(int ci)
     } else if(header->error != 0) {
 	log_error("Header contined error: %d", header->error);
     }
-    
-    header = realloc(header, header->size);
+
+    h_new = realloc(header, header->size);
+    if (h_new == NULL)
+	goto bail;
+    header = h_new;
+
     /* Use a char* so we can store the remainder into an offset */
     data = (char*)header;

@@ -252,6 +258,7 @@ void process_cluster(int ci)
     goto done;
     
   bail:
+    free (header);
     log_error("AIS connection failed");
     return;
 }
@@ -408,4 +415,3 @@ int fence_in_progress(int *count)
 {
 	return 0;
 }
-
-- 
1.6.3.3.420.gd4b46