All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Rockai <prockai@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH 08/29] Fix memory leak in error path
Date: Fri, 26 Nov 2010 08:37:52 +0100	[thread overview]
Message-ID: <87pqtslelr.fsf@twilight.int.mornfall.net.> (raw)
In-Reply-To: <fe9b9e08dfad635aff060d826257f24ce49dd23b.1290682013.git.zkabelac@redhat.com> (Zdenek Kabelac's message of "Thu, 25 Nov 2010 11:55:12 +0100")

Zdenek Kabelac <zkabelac@redhat.com> writes:

> Nicely hidden memory leak in error path.
> outf() is macro which uses out_text() and does automagical return_0.
> This would leak tag_buffer.
>
> As there was a repeating code for tags output - create _out_tags().
>
> Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
Ack.

(I haven't tried the patch, but as long as it compiles and passes
testsuite, it should be good to go.)

> --- a/lib/format_text/export.c
> +++ b/lib/format_text/export.c
> @@ -363,10 +363,27 @@ static int _print_flag_config(struct formatter *f, uint64_t status, int type)
>  	return 1;
>  }
>  
> +
> +static int _out_tags(struct formatter *f, struct dm_list *tags)
> +{
> +	char *tag_buffer;
> +
> +	if (!dm_list_empty(tags)) {
> +		if (!(tag_buffer = alloc_printed_tags(tags)))
> +			return_0;
> +		if (!out_text(f, "tags = %s", tag_buffer)) {
> +			dm_free(tag_buffer);
> +			return_0;
> +		}
> +		dm_free(tag_buffer);
> +	}
> +
> +	return 1;
> +}
> +
>  static int _print_vg(struct formatter *f, struct volume_group *vg)
>  {
>  	char buffer[4096];
> -	char *tag_buffer = NULL;
>  
>  	if (!id_write_format(&vg->id, buffer, sizeof(buffer)))
>  		return_0;
> @@ -378,12 +395,8 @@ static int _print_vg(struct formatter *f, struct volume_group *vg)
>  	if (!_print_flag_config(f, vg->status, VG_FLAGS))
>  		return_0;
>  
> -	if (!dm_list_empty(&vg->tags)) {
> -		if (!(tag_buffer = alloc_printed_tags(&vg->tags)))
> -			return_0;
> -		outf(f, "tags = %s", tag_buffer);
> -		dm_free(tag_buffer);
> -	}
> +	if (!_out_tags(f, &vg->tags))
> +		return_0;
>  
>  	if (vg->system_id && *vg->system_id)
>  		outf(f, "system_id = \"%s\"", vg->system_id);
> @@ -428,7 +441,7 @@ static int _print_pvs(struct formatter *f, struct volume_group *vg)
>  	struct pv_list *pvl;
>  	struct physical_volume *pv;
>  	char buffer[4096];
> -	char *buf, *tag_buffer = NULL;
> +	char *buf;
>  	const char *name;
>  
>  	outf(f, "physical_volumes {");
> @@ -462,12 +475,8 @@ static int _print_pvs(struct formatter *f, struct volume_group *vg)
>  		if (!_print_flag_config(f, pv->status, PV_FLAGS))
>  			return_0;
>  
> -		if (!dm_list_empty(&pv->tags)) {
> -			if (!(tag_buffer = alloc_printed_tags(&pv->tags)))
> -				return_0;
> -			outf(f, "tags = %s", tag_buffer);
> -			dm_free(tag_buffer);
> -		}
> +		if (!_out_tags(f, &pv->tags))
> +			return_0;
>  
>  		outsize(f, pv->size, "dev_size = %" PRIu64, pv->size);
>  
> @@ -487,8 +496,6 @@ static int _print_pvs(struct formatter *f, struct volume_group *vg)
>  static int _print_segment(struct formatter *f, struct volume_group *vg,
>  			  int count, struct lv_segment *seg)
>  {
> -	char *tag_buffer = NULL;
> -
>  	outf(f, "segment%u {", count);
>  	_inc_indent(f);
>  
> @@ -499,12 +506,8 @@ static int _print_segment(struct formatter *f, struct volume_group *vg,
>  	outnl(f);
>  	outf(f, "type = \"%s\"", seg->segtype->name);
>  
> -	if (!dm_list_empty(&seg->tags)) {
> -		if (!(tag_buffer = alloc_printed_tags(&seg->tags)))
> -			return_0;
> -		outf(f, "tags = %s", tag_buffer);
> -		dm_free(tag_buffer);
> -	}
> +	if (!_out_tags(f, &seg->tags))
> +		return_0;
>  
>  	if (seg->segtype->ops->text_export &&
>  	    !seg->segtype->ops->text_export(seg, f))
> @@ -557,7 +560,6 @@ static int _print_lv(struct formatter *f, struct logical_volume *lv)
>  {
>  	struct lv_segment *seg;
>  	char buffer[4096];
> -	char *tag_buffer = NULL;
>  	int seg_count;
>  
>  	outnl(f);
> @@ -573,12 +575,8 @@ static int _print_lv(struct formatter *f, struct logical_volume *lv)
>  	if (!_print_flag_config(f, lv->status, LV_FLAGS))
>  		return_0;
>  
> -	if (!dm_list_empty(&lv->tags)) {
> -		if (!(tag_buffer = alloc_printed_tags(&lv->tags)))
> -			return_0;
> -		outf(f, "tags = %s", tag_buffer);
> -		dm_free(tag_buffer);
> -	}
> +	if (!_out_tags(f, &lv->tags))
> +		return_0;
>  
>  	if (lv->alloc != ALLOC_INHERIT)
>  		outf(f, "allocation_policy = \"%s\"",



  reply	other threads:[~2010-11-26  7:37 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-25 10:55 [PATCH 00/29] Fixes for analyzer problems Zdenek Kabelac
2010-11-25 10:55 ` [PATCH 01/29] Cleanup remove test for NULL Zdenek Kabelac
2010-11-25 17:18   ` Petr Rockai
2010-11-25 10:55 ` [PATCH 02/29] Fix check for empty system_dir Zdenek Kabelac
2010-11-25 17:19   ` Petr Rockai
2010-11-25 23:12     ` Zdenek Kabelac
2010-11-26  7:35       ` Petr Rockai
2010-11-29 23:27       ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 03/29] Remove printing of LCK_CACHE Zdenek Kabelac
2010-11-25 17:26   ` Petr Rockai
2010-11-29 20:44   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 04/29] Reset vg pointer after release Zdenek Kabelac
2010-11-25 17:26   ` Petr Rockai
2010-11-25 10:55 ` [PATCH 05/29] Test *buf for NULL Zdenek Kabelac
2010-11-29 20:04   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 06/29] Replace snprintf -> dm_snprintf Zdenek Kabelac
2010-11-29 20:16   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 07/29] Test success from dm_poll_create Zdenek Kabelac
2010-11-29 20:11   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 08/29] Fix memory leak in error path Zdenek Kabelac
2010-11-26  7:37   ` Petr Rockai [this message]
2010-11-25 10:55 ` [PATCH 09/29] Remove check for lv is NULL Zdenek Kabelac
2010-11-29 20:09   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 10/29] Add some backtrace - Attention please Zdenek Kabelac
2010-11-29 20:16   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 11/29] Add stack trace for error path Zdenek Kabelac
2010-11-29 20:43   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 12/29] Add test for 'read' result Zdenek Kabelac
2010-11-29 20:19   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 13/29] Put some FIXME warnings in lvmcache_update_vg processing Zdenek Kabelac
2010-12-21 15:15   ` Zdenek Kabelac
2010-11-25 10:55 ` [PATCH 14/29] Remove unneeded check for NULL pvd->system_id Zdenek Kabelac
2010-11-29 20:21   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 15/29] Modify test to catch passing NULL pointer Zdenek Kabelac
2010-11-29 21:17   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 16/29] Test uuid for NULL Zdenek Kabelac
2010-11-29 21:00   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 17/29] Optimize second call to strchr with same parameters Zdenek Kabelac
2010-11-29 20:50   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 18/29] Check result of vginfo_from_vgname Zdenek Kabelac
2010-11-29 20:56   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 19/29] Test for error status Zdenek Kabelac
2010-11-29 21:02   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 20/29] Add test for lv_name not NULL Zdenek Kabelac
2010-11-29 21:21   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 21/29] Instrument with nonnull dev_manager_transient Zdenek Kabelac
2010-11-29 21:51   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 22/29] Ensure pointer first is notnull before dereference Zdenek Kabelac
2010-11-29 21:56   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 23/29] Add test and error message for failure case Zdenek Kabelac
2010-11-29 21:18   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 24/29] Test for str_list_add Zdenek Kabelac
2010-11-29 21:19   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 25/29] Check for unlink result Zdenek Kabelac
2010-11-29 21:50   ` Alasdair G Kergon
2010-11-29 21:51   ` Alasdair G Kergon
2010-12-21 15:04     ` Zdenek Kabelac
2010-11-25 10:55 ` [PATCH 26/29] Add stack traces for dev_set/close_immediate error path Zdenek Kabelac
2010-11-29 21:22   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 27/29] Add standard check for result of lv_info call Zdenek Kabelac
2010-11-25 16:24   ` Zdenek Kabelac
2010-11-29 21:34   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 28/29] Check type is not NULL before access Zdenek Kabelac
2010-11-29 21:38   ` Alasdair G Kergon
2010-11-25 10:55 ` [PATCH 29/29] Check for NULL pointer Zdenek Kabelac
2010-11-25 23:02   ` Zdenek Kabelac
2010-11-29 22:47     ` Alasdair G Kergon
2010-11-30 12:33       ` Zdenek Kabelac
2010-11-30 13:07         ` Alasdair G Kergon
2010-11-29 21:43   ` Alasdair G Kergon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pqtslelr.fsf@twilight.int.mornfall.net. \
    --to=prockai@redhat.com \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.