Hi Mahesh,
Thank you for checking the above also.
commit 92d2934f9e1694e7195d93f9816fbea8a657f05f fixes them.
Thanks
Ken'ichi Ohmichi
On Thu, 11 Aug 2011 17:03:06 +0530
Mahesh Jagannath Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
>
> Hi Ken'ichi,
>
> On 08/11/2011 07:37 AM, Ken'ichi Ohmichi wrote:
> >
> > Hi Mahesh,
> >
> > On Wed, 18 May 2011 01:34:15 +0530
> > Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
> >>
> >> This patch enables to makedumpfile to read and process filter commands from
> >> specified config file. It builds a list of filter info consisting of memory
> >> address (paddr) and size to be filtered out. This list is used during
> >> read_pfn() function to filter out user specified kernel data from vmcore
> >> before writing it to compressed DUMPFILE. The filtered memory locations are
> >> filled with 'X' (0x58) character.
> >>
> >> The filter command syntax is:
> >> erase <Symbol>[.member[...]] [size <SizeValue>[K|M]]
> >> erase <Symbol>[.member[...]] [size <SizeSymbol>]
> >> erase <Symbol>[.member[...]] [nullify]
> >>
> >> Below are the examples how filter commands in config file look like:
> >>
> >> erase modules
> >> erase cred_jar.name size 10
> >> erase vmlist.addr nullify
> >
> > Thank you for the work.
> > I tested this feature by the above example, and confirmed the feature
> > works fine. Nice work :-)
>
> Thanks :-)
>
> >
> > There are some comments in the below lines.
> > Can you check them ?
> >
> >> +/*
> >> + * Read the non-terminal's which are in the form of <Symbol>[.member[...]]
> >> + */
> >> +struct config_entry *
> >> +create_config_entry(const char *token, unsigned short flag, int line)
> >> +{
> >> + struct config_entry *ce = NULL, *ptr, *prev_ce;
> >> + char *str, *cur, *next;
> >> + long len;
> >> + int depth = 0;
> >> +
> >> + if (!token)
> >> + return NULL;
> >> +
> >> + cur = str = strdup(token);
> >> + prev_ce = ptr = NULL;
> >> + while (cur != NULL) {
> >> + if ((next = strchr(cur, '.')) != NULL) {
> >> + *next++ = '\0';
> >> + }
> >> + if (!strlen(cur)) {
> >> + cur = next;
> >> + continue;
> >> + }
> >> +
> >> + if ((ptr = calloc(1, sizeof(struct config_entry))) == NULL) {
> >> + ERRMSG("Can't allocate memory for config_entry\n");
> >> + goto err_out;
> >> + }
> >> + ptr->line = line;
> >> + ptr->flag |= flag;
> >> + if (depth == 0) {
> >> + /* First node is always a symbol name */
> >> + ptr->flag |= SYMBOL_ENTRY;
> >> + }
> >> + if (flag & FILTER_ENTRY) {
> >> + ptr->name = strdup(cur);
> >> + }
> >> + if (flag & SIZE_ENTRY) {
> >> + char ch = '\0';
> >> + int n = 0;
> >> + /* See if absolute length is provided */
> >> + if ((depth == 0) &&
> >> + ((n = sscanf(cur, "%zd%c", &len, &ch)) > 0)) {
> >> + if (len < 0) {
> >> + ERRMSG("Config error at %d: size "
> >> + "value must be positive.\n",
> >> + line);
> >> + goto err_out;
> >> + }
> >> + ptr->size = len;
> >> + ptr->flag |= ENTRY_RESOLVED;
> >> + if (n == 2) {
> >> + /* Handle suffix.
> >> + * K = Kilobytes
> >> + * M = Megabytes
> >> + */
> >> + switch (ch) {
> >> + case 'M':
> >> + case 'm':
> >> + ptr->size *= 1024;
> >> + case 'K':
> >> + case 'k':
> >> + ptr->size *= 1024;
> >> + break;
> >> + }
> >> + }
> >> + }
> >> + else
> >> + ptr->name = strdup(cur);
> >> + }
> >> + if (prev_ce) {
> >> + prev_ce->next = ptr;
> >> + prev_ce = ptr;
> >> + }
> >> + else
> >> + ce = prev_ce = ptr;
> >> + cur = next;
> >> + depth++;
> >> + ptr = NULL;
> >> + }
> >> + free(str);
> >> + return ce;
> >> +
> >> +err_out:
> >> + if (ce)
> >> + free_config_entry(ce);
> >> + if (ptr)
> >> + free_config_entry(ptr);
> >
> > "free(str);" is necessary.
> >
>
> Nice catch. Agree.
>
> >> + return NULL;
> >> +}
> >
> > [..]
> >
> >> +/*
> >> + * read filter config file and return each string token. If the parameter
> >> + * expected_token is non-NULL, then return the current token if it matches
> >> + * with expected_token otherwise save the current token and return NULL.
> >> + * At start of every module section filter_config.new_section is set to 1 and
> >> + * subsequent function invocations return NULL untill filter_config.new_section
> >> + * is reset to 0 by passing @flag = CONFIG_NEW_CMD (0x02).
> >> + *
> >> + * Parameters:
> >> + * @expected_token INPUT
> >> + * Token string to match with currnet token.
> >> + * =NULL - return the current available token.
> >> + *
> >> + * @flag INPUT
> >> + * =0x01 - Skip to next module section.
> >> + * =0x02 - Treat the next token as next filter command and reset.
> >> + *
> >> + * @line OUTPUT
> >> + * Line number of current token in filter config file.
> >> + *
> >> + * @cur_mod OUTPUT
> >> + * Points to current module section name on non-NULL return value.
> >> + *
> >> + * @eof OUTPUT
> >> + * set to -1 when end of file is reached.
> >> + * set to -2 when end of section is reached.
> >> + */
> >> +static char *
> >> +get_config_token(char *expected_token, unsigned char flag, int *line,
> >> + char **cur_mod, int *eof)
> >> +{
> >> + char *p;
> >> + struct filter_config *fc = &filter_config;
> >> + int skip = flag & CONFIG_SKIP_SECTION;
> >
> > CONFIG_SKIP_SECTION is used only here, and we need to fix get_config()
> > for the readability. Please check my comment in get_config().
> >
> > [..]
> >
> >> +/*
> >> + * Configuration file 'makedumpfile.conf' contains filter commands.
> >> + * Every individual filter command is considered as a config entry. A config
> >> + * entry can be provided on a single line or multiple lines.
> >> + */
> >> +struct config *
> >> +get_config(int skip)
> >> +{
> >> + struct config *config;
> >> + char *token = NULL;
> >> + static int line_count = 0;
> >> + char *cur_module = NULL;
> >> + int eof = 0;
> >> + unsigned char flag = CONFIG_NEW_CMD | skip;
> >
> > We need to change the above:
> >
> > unsigned char flag = CONFIG_NEW_CMD;
> > if (skip)
> > flag |= CONFIG_SKIP_SECTION;
> >
>
> Agree.
>
> Thanks,
> -Mahesh.
>
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
