[PATCH v2 3/8] makedumpfile: Load the module symbol data from vmcore.

[Ken'ichi Ohmichi]

Hi Mehesh,

On Wed, 03 Aug 2011 16:05:38 +0530
Mahesh Jagannath Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
> > 
> >> +static int
> >> +load_module_symbols(void)
> >> +{
> >> +	unsigned long head, cur, cur_module;
> >> +	unsigned long symtab, strtab;
> >> +	unsigned long mod_base, mod_init;
> >> +	unsigned int mod_size, mod_init_size;
> >> +	unsigned char *module_struct_mem, *module_core_mem;
> >> +	unsigned char *module_init_mem = NULL;
> >> +	unsigned char *symtab_mem;
> >> +	char *module_name, *strtab_mem, *nameptr;
> >> +	struct module_info *modules = NULL;
> >> +	struct symbol_info *sym_info;
> >> +	unsigned int num_symtab;
> >> +	unsigned int i = 0, nsym;
> >> +
> >> +	head = SYMBOL(modules);
> >> +	if (!get_num_modules(head, &mod_st.num_modules)) {
> >> +		ERRMSG("Can't get module count\n");
> >> +		return FALSE;
> >> +	}
> >> +	if (!mod_st.num_modules) {
> >> +		return FALSE;
> > 
> > If the above num_modules is 0, makedumpfile fails without any hint
> > message. How about the below change ?
> 
> Agree.

[..]

> >> +		if (mod_init_size > 0) {
> >> +			module_init_mem = calloc(1, mod_init_size);
> >> +			if (module_init_mem == NULL) {
> >> +				ERRMSG("Can't allocate memory for module "
> >> +								"init\n");
> >> +				return FALSE;
> >> +			}
> >> +			if (!readmem(VADDR, mod_init, module_init_mem,
> >> +							mod_init_size)) {
> >> +				ERRMSG("Can't access module init in memory.\n");
> >> +				return FALSE;
> > 
> > In the above error case, module_init_mem should be freed.
> > There are the same lacks of free, and I feel it is due to
> > a large load_module_symbols() function.
> > Hence I created the attached patch for making the function
> > small and fixing the lacks of free.
> > Can you review it ?
> 
> The attached patch looks good to me. Thanks for splitting the function.

Thank you for your review of two patches.
I have merged them into filter-out-devel branch.


Thanks
Ken'ichi Ohmichi