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