public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Manish Katiyar <mkatiyar@gmail.com>
Cc: mingo@elte.hu, LKML <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] Remove errors caught by checkpatch.pl in kernel/kallsyms.c
Date: Sun, 15 Feb 2009 19:47:52 +0100	[thread overview]
Message-ID: <20090215184752.GA4970@uranus.ravnborg.org> (raw)
In-Reply-To: <ea11fea30902151034s5444efabw455c1dac1b875086@mail.gmail.com>

On Mon, Feb 16, 2009 at 12:04:36AM +0530, Manish Katiyar wrote:
> Hi Ingo,
> 
> I used your code-quality script to do cleanup in kernel/kallsyms.c.
> Below patch removes errors generated by checkpatch.pl.
When doing so use checkpatch only as a hint generator and do
not concentrate only on the warnings/errors generated by checkpatch.

Your patch is an improvement but please fix the remaining issues.

	Sam

For example..
> -	return kallsyms_token_table[ kallsyms_token_index[ kallsyms_names[off+1] ] ];
> +	return kallsyms_token_table[kallsyms_token_index[kallsyms_names[off+1]]];

Add spaces around [off+1] so it looks like this: [off + 1]


>  }
> 
> 
> @@ -133,13 +133,13 @@ static unsigned int get_symbol_offset(unsigned long pos)
> 
>  	/* use the closest marker we have. We have markers every 256 positions,
>  	 * so that should be close enough */
> -	name = &kallsyms_names[ kallsyms_markers[pos>>8] ];
> +	name = &kallsyms_names[kallsyms_markers[pos>>8]];

Same here: [pos >> 8]


> 
>  	/* sequentially scan all the symbols up to the point we're searching for.
>  	 * Every symbol is stored in a [<len>][<len> bytes of data] format, so we
>  	 * just need to add the len to the current pointer for every symbol we
>  	 * wish to skip */
> -	for(i = 0; i < (pos&0xFF); i++)
> +	for (i = 0; i < (pos&0xFF); i++)
And here: (pos & 0xFF)

>  		name = name + (*name) + 1;
> 
>  	return name - kallsyms_names;
> @@ -323,6 +323,7 @@ int sprint_symbol(char *buffer, unsigned long address)
> 
>  	return len;
>  }
> +EXPORT_SYMBOL_GPL(sprint_symbol);
> 
>  /* Look up a kernel symbol and print it to the kernel messages. */
>  void __print_symbol(const char *fmt, unsigned long address)
> @@ -333,10 +334,10 @@ void __print_symbol(const char *fmt, unsigned
> long address)
> 
>  	printk(fmt, buffer);
>  }
> +EXPORT_SYMBOL(__print_symbol);
> 
>  /* To avoid using get_symbol_offset for every symbol, we carry prefix along. */
> -struct kallsym_iter
> -{
> +struct kallsym_iter {
>  	loff_t pos;
>  	unsigned long value;
>  	unsigned int nameoff; /* If iterating in core kernel symbols */
> @@ -385,7 +386,7 @@ static int update_iter(struct kallsym_iter *iter,
> loff_t pos)
>  		iter->pos = pos;
>  		return get_ksymbol_mod(iter);
>  	}
> -	
> +
>  	/* If we're not on the desired position, reset to new position. */
>  	if (pos != iter->pos)
>  		reset_iter(iter, pos);
> @@ -420,7 +421,7 @@ static int s_show(struct seq_file *m, void *p)
>  {
>  	struct kallsym_iter *iter = m->private;
> 
> -	/* Some debugging symbols have no name.  Ignore them. */
> +	/* Some debugging symbols have no name.  Ignore them. */
>  	if (!iter->name[0])
>  		return 0;
> 
> @@ -432,11 +433,11 @@ static int s_show(struct seq_file *m, void *p)
>  		type = iter->exported ? toupper(iter->type) :
>  					tolower(iter->type);
>  		seq_printf(m, "%0*lx %c %s\t[%s]\n",
> -			   (int)(2*sizeof(void*)),
> +			   (int)(2*sizeof(void *)),
(2 * sizeof(void *))


>  			   iter->value, type, iter->name, iter->module_name);
>  	} else
>  		seq_printf(m, "%0*lx %c %s\n",
> -			   (int)(2*sizeof(void*)),
> +			   (int)(2*sizeof(void *)),
and here again.

>  			   iter->value, iter->type, iter->name);
>  	return 0;
>  }
> @@ -481,7 +482,5 @@ static int __init kallsyms_init(void)
>  	proc_create("kallsyms", 0444, NULL, &kallsyms_operations);
>  	return 0;
>  }
> -__initcall(kallsyms_init);
> +device_initcall(kallsyms_init);
> 
> -EXPORT_SYMBOL(__print_symbol);
> -EXPORT_SYMBOL_GPL(sprint_symbol);


  reply	other threads:[~2009-02-15 18:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-15 18:34 [PATCH] Remove errors caught by checkpatch.pl in kernel/kallsyms.c Manish Katiyar
2009-02-15 18:47 ` Sam Ravnborg [this message]
2009-02-15 18:47   ` Manish Katiyar
2009-02-16 13:07   ` Stefan Richter
2009-02-16 13:28     ` Ingo Molnar
2009-02-16 14:00       ` Stefan Richter
2009-02-16 14:19         ` Ingo Molnar
2009-02-16 15:22           ` Stefan Richter
2009-02-16 15:41             ` Manish Katiyar
2009-02-16 15:50             ` Ingo Molnar
2009-02-16 16:13               ` Stefan Richter
2009-02-16 17:12                 ` Ingo Molnar
2009-02-16 18:04                   ` Stefan Richter
2009-02-16 16:13               ` Al Viro
2009-02-16 17:11                 ` Ingo Molnar
2009-02-16 14:28       ` Paolo Ciarrocchi
2009-02-16 16:17       ` Julia Lawall
2009-02-16 16:35         ` Stefan Richter
2009-02-16 17:21           ` Ingo Molnar
2009-02-16 17:15         ` Ingo Molnar

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=20090215184752.GA4970@uranus.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mkatiyar@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox