[Ocfs2-tools-devel] [PATCH] mkfs.ocfs2: fix memory leak problems in main()

[Eric Ren]

Hi again,

On 08/25/2016 04:40 PM, piaojun wrote:
> 'root_dir', 'system_dir', 'orphan_dir', 'record' and 's' need to be
> freed at the end of main().
>
> Signed-off-by: Jun Piao <piaojun at huawei.com>
> ---
>   mkfs.ocfs2/mkfs.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
>
> diff --git a/mkfs.ocfs2/mkfs.c b/mkfs.ocfs2/mkfs.c
> index c361fda..321ba67 100644
> --- a/mkfs.ocfs2/mkfs.c
> +++ b/mkfs.ocfs2/mkfs.c
> @@ -21,6 +21,7 @@
>   #include "mkfs.h"
>   
>   static State *get_state(int argc, char **argv);
> +static void free_state(State *s);
>   static int get_number(char *arg, uint64_t *res);
>   static void parse_journal_opts(char *progname, const char *opts,
>   			       uint64_t *journal_size_in_bytes,
> @@ -46,6 +47,7 @@ static int alloc_from_bitmap(State *s, uint64_t num_bits, AllocBitmap *bitmap,
>   			     uint64_t *start, uint64_t *num);
>   static uint64_t alloc_inode(State *s, uint16_t *suballoc_bit);
>   static DirData *alloc_directory(State *s);
> +static void free_directory(DirData *dir);
>   static void add_entry_to_directory(State *s, DirData *dir, char *name,
>   				   uint64_t byte_off, uint8_t type);
>   static uint32_t blocks_needed(State *s);
> @@ -76,6 +78,7 @@ static AllocGroup * initialize_alloc_group(State *s, const char *name,
>   					   uint64_t blkno,
>   					   uint16_t chain, uint16_t cpg,
>   					   uint16_t bpc);
> +static void free_alloc_group(AllocGroup *group);
>   static void index_system_dirs(State *s, ocfs2_filesys *fs);
>   static void create_lost_found_dir(State *s, ocfs2_filesys *fs);
>   static void format_journals(State *s, ocfs2_filesys *fs);
> @@ -823,6 +826,16 @@ main(int argc, char **argv)
>   	if (!s->quiet)
>   		printf("%s successful\n\n", s->progname);
>   
> +	free_directory(root_dir);
> +	free_directory(system_dir);
> +	for (i = 0; i < s->initial_slots; i++)
> +		free_directory(orphan_dir[i]);
> +
> +	for (i = 0; i < NUM_SYSTEM_INODES; i++)
> +		ocfs2_free(&record[i]);
> +
> +	free_state(s);
> +
>   	return 0;
>   }
>   
> @@ -1227,6 +1240,25 @@ get_state(int argc, char **argv)
>   	return s;
>   }
>   
> +static void
> +free_state(State *s)
> +{
> +	int i;
> +
> +	ocfs2_free(&s->vol_label);
> +	ocfs2_free(&s->device_name);
> +	ocfs2_free(&s->cluster_stack);
> +	ocfs2_free(&s->cluster_name);

Possibly, s->progname may hit being assigned strdup("mkfs.ocfs2").

> +
> +	for (i = 0; i < s->nr_cluster_groups; i++)
> +		free_alloc_group(s->global_bm->groups[i]);
> +	ocfs2_free(&s->global_bm->groups);
> +	ocfs2_free(&s->global_bm->name);
> + 	ocfs2_free(&s->global_bm);

This seems to be the complex part;-) So, how about
_AllocBitmp::bm_record,
_AllocBitmp::alloc_record,
_AllocGroup::alloc_inode?

TBH, i cannot figure this out clearly;-)

Thanks,
Eric

> +
> +	free_alloc_group(s->system_group);
> +}
> +
>   static int
>   get_number(char *arg, uint64_t *res)
>   {
> @@ -1841,6 +1873,14 @@ initialize_alloc_group(State *s, const char *name,
>   	return group;
>   }
>   
> +static void
> +free_alloc_group(AllocGroup *group)
> +{
> +	ocfs2_free(&group->name);
> +	ocfs2_free(&group->gd);
> +	ocfs2_free(&group);
> +}
> +
>   static AllocBitmap *
>   initialize_bitmap(State *s, uint32_t bits, uint32_t unit_bits,
>   		  const char *name, SystemFileDiskRecord *bm_record)
> @@ -2129,6 +2169,13 @@ alloc_directory(State *s)
>   }
>   
>   static void
> +free_directory(DirData *dir)
> +{
> +	ocfs2_free(&dir->buf);
> +	ocfs2_free(&dir);
> +}
> +
> +static void
>   add_entry_to_directory(State *s, DirData *dir, char *name, uint64_t byte_off,
>   		       uint8_t type)
>   {