linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()
@ 2011-05-23  1:19 Eduardo Silva
  2011-05-23  1:36 ` H. Peter Anvin
  0 siblings, 1 reply; 6+ messages in thread
From: Eduardo Silva @ 2011-05-23  1:19 UTC (permalink / raw)
  To: Steven Rostedt, Frederic Weisbecker, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin, x86
  Cc: linux-kernel

the function ftrace_nop_replace() returns a 'static const unsigned char *'
value, so when the caller perform a direct assignment to a
'static unsigned char *', the compiler raise the following warning message:

 ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type
 ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type

Adding the proper casts the message goes away.

Signed-off-by: Eduardo Silva <edsiper@gmail.com>
---
 arch/x86/kernel/ftrace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 0ba15a6..b44c3c9 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -305,7 +305,7 @@ int ftrace_make_nop(struct module *mod,
 	unsigned long ip = rec->ip;
 
 	old = ftrace_call_replace(ip, addr);
-	new = ftrace_nop_replace();
+	new = (unsigned char *) ftrace_nop_replace();
 
 	return ftrace_modify_code(rec->ip, old, new);
 }
@@ -315,7 +315,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	unsigned char *new, *old;
 	unsigned long ip = rec->ip;
 
-	old = ftrace_nop_replace();
+	old = (unsigned char *) ftrace_nop_replace();
 	new = ftrace_call_replace(ip, addr);
 
 	return ftrace_modify_code(rec->ip, old, new);
-- 
1.7.4.1




^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()
  2011-05-23  1:19 [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace() Eduardo Silva
@ 2011-05-23  1:36 ` H. Peter Anvin
  2011-05-23  2:27   ` Eduardo Silva
  2011-05-23 13:08   ` Steven Rostedt
  0 siblings, 2 replies; 6+ messages in thread
From: H. Peter Anvin @ 2011-05-23  1:36 UTC (permalink / raw)
  To: Eduardo Silva
  Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar, Thomas Gleixner,
	x86, linux-kernel

On 05/22/2011 06:19 PM, Eduardo Silva wrote:
> the function ftrace_nop_replace() returns a 'static const unsigned char *'
> value, so when the caller perform a direct assignment to a
> 'static unsigned char *', the compiler raise the following warning message:
> 
>  ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type
>  ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type
> 
> Adding the proper casts the message goes away.
> 
> Signed-off-by: Eduardo Silva <edsiper@gmail.com>

This quiets a warning of something that potentially looks like a real
bug.  Perhaps "new" should be const?

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()
  2011-05-23  1:36 ` H. Peter Anvin
@ 2011-05-23  2:27   ` Eduardo Silva
  2011-05-23 13:08   ` Steven Rostedt
  1 sibling, 0 replies; 6+ messages in thread
From: Eduardo Silva @ 2011-05-23  2:27 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Steven Rostedt, Frederic Weisbecker, Ingo Molnar, Thomas Gleixner,
	x86, linux-kernel

On Sun, May 22, 2011 at 9:36 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/22/2011 06:19 PM, Eduardo Silva wrote:
>> the function ftrace_nop_replace() returns a 'static const unsigned char *'
>> value, so when the caller perform a direct assignment to a
>> 'static unsigned char *', the compiler raise the following warning message:
>>
>>  ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type
>>  ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type
>>
>> Adding the proper casts the message goes away.
>>
>> Signed-off-by: Eduardo Silva <edsiper@gmail.com>
>
> This quiets a warning of something that potentially looks like a real
> bug.  Perhaps "new" should be const?

The value is returned in the following function:

static const unsigned char *ftrace_nop_replace(void)
{
	return ideal_nops[NOP_ATOMIC5];
}

ideal_nops seems to be constant and architecture dependent, so make
sense to return it as a constant pointer... i think that we need more
comments about it.

regards,



>
>        -hpa
>
>
> --
> H. Peter Anvin, Intel Open Source Technology Center
> I work for Intel.  I don't speak on their behalf.
>
>



-- 
Eduardo Silva
http://edsiper.linuxchile.cl
http://www.monkey-project.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()
  2011-05-23  1:36 ` H. Peter Anvin
  2011-05-23  2:27   ` Eduardo Silva
@ 2011-05-23 13:08   ` Steven Rostedt
  2011-05-23 20:01     ` John Kacur
  1 sibling, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2011-05-23 13:08 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Eduardo Silva, Frederic Weisbecker, Ingo Molnar, Thomas Gleixner,
	x86, linux-kernel

On Sun, 2011-05-22 at 18:36 -0700, H. Peter Anvin wrote:
> On 05/22/2011 06:19 PM, Eduardo Silva wrote:
> > the function ftrace_nop_replace() returns a 'static const unsigned char *'
> > value, so when the caller perform a direct assignment to a
> > 'static unsigned char *', the compiler raise the following warning message:
> > 
> >  ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type
> >  ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type
> > 
> > Adding the proper casts the message goes away.
> > 
> > Signed-off-by: Eduardo Silva <edsiper@gmail.com>
> 
> This quiets a warning of something that potentially looks like a real
> bug.  Perhaps "new" should be const?

Although there is no real bug here, I hate these "hide the warning"
patches.

The real solution is to change all the users into const *. This should
be doable as the new and old pointers are not modified.

And remember:

	char *changes;
	const char *no_changes;

	no_changes = changes;  /* is OK! (no compiler warnings) */

	changes = no_changes; /* is not OK. (compiler will warn). */

-- Steve



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()
  2011-05-23 13:08   ` Steven Rostedt
@ 2011-05-23 20:01     ` John Kacur
  2011-05-23 20:05       ` H. Peter Anvin
  0 siblings, 1 reply; 6+ messages in thread
From: John Kacur @ 2011-05-23 20:01 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: H. Peter Anvin, Eduardo Silva, Frederic Weisbecker, Ingo Molnar,
	Thomas Gleixner, x86, linux-kernel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4007 bytes --]



On Mon, 23 May 2011, Steven Rostedt wrote:

> On Sun, 2011-05-22 at 18:36 -0700, H. Peter Anvin wrote:
> > On 05/22/2011 06:19 PM, Eduardo Silva wrote:
> > > the function ftrace_nop_replace() returns a 'static const unsigned char *'
> > > value, so when the caller perform a direct assignment to a
> > > 'static unsigned char *', the compiler raise the following warning message:
> > > 
> > >  ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type
> > >  ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type
> > > 
> > > Adding the proper casts the message goes away.
> > > 
> > > Signed-off-by: Eduardo Silva <edsiper@gmail.com>
> > 
> > This quiets a warning of something that potentially looks like a real
> > bug.  Perhaps "new" should be const?
> 
> Although there is no real bug here, I hate these "hide the warning"
> patches.
> 
> The real solution is to change all the users into const *. This should
> be doable as the new and old pointers are not modified.
> 
> And remember:
> 
> 	char *changes;
> 	const char *no_changes;
> 
> 	no_changes = changes;  /* is OK! (no compiler warnings) */
> 
> 	changes = no_changes; /* is not OK. (compiler will warn). */
> 
> -- Steve

The following needs some testing, just compile tested for now.

>From 71d6e5971cc90a03fd2d7bf8aaa2b81932bd0c7c Mon Sep 17 00:00:00 2001
From: John Kacur <jkacur@redhat.com>
Date: Mon, 23 May 2011 21:24:27 +0200
Subject: [PATCH] ftrace: Change unsigned char *new, *old to const
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This removes the warning:
arch/x86/kernel/ftrace.c: In function ‘ftrace_make_nop’:
/arch/x86/kernel/ftrace.c:308: warning: assignment discards qualifiers from pointer target type

Steven Rostedt suggested to make these pointers const pointers.

This patch assumes that
https://patchwork.kernel.org/patch/798172/
is applied first.

Reported-by: Eduardo Silva <edsiper@gmail.com>
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 arch/x86/kernel/ftrace.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 0ba15a6..e4f23d1 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -123,7 +123,7 @@ static unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
 static atomic_t nmi_running = ATOMIC_INIT(0);
 static int mod_code_status;		/* holds return value of text write */
 static void *mod_code_ip;		/* holds the IP to write to */
-static void *mod_code_newcode;		/* holds the text to write to the IP */
+static const void *mod_code_newcode;	/* holds the text to write to the IP */
 
 static unsigned nmi_wait_count;
 static atomic_t nmi_update_count = ATOMIC_INIT(0);
@@ -225,7 +225,7 @@ within(unsigned long addr, unsigned long start, unsigned long end)
 }
 
 static int
-do_ftrace_mod_code(unsigned long ip, void *new_code)
+do_ftrace_mod_code(unsigned long ip, const void *new_code)
 {
 	/*
 	 * On x86_64, kernel text mappings are mapped read-only with
@@ -266,8 +266,8 @@ static const unsigned char *ftrace_nop_replace(void)
 }
 
 static int
-ftrace_modify_code(unsigned long ip, unsigned char *old_code,
-		   unsigned char *new_code)
+ftrace_modify_code(unsigned long ip, const unsigned char *old_code,
+		   const unsigned char *new_code)
 {
 	unsigned char replaced[MCOUNT_INSN_SIZE];
 
@@ -301,7 +301,7 @@ ftrace_modify_code(unsigned long ip, unsigned char *old_code,
 int ftrace_make_nop(struct module *mod,
 		    struct dyn_ftrace *rec, unsigned long addr)
 {
-	unsigned char *new, *old;
+	const unsigned char *new, *old;
 	unsigned long ip = rec->ip;
 
 	old = ftrace_call_replace(ip, addr);
@@ -312,7 +312,7 @@ int ftrace_make_nop(struct module *mod,
 
 int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 {
-	unsigned char *new, *old;
+	const unsigned char *new, *old;
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace();
-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()
  2011-05-23 20:01     ` John Kacur
@ 2011-05-23 20:05       ` H. Peter Anvin
  0 siblings, 0 replies; 6+ messages in thread
From: H. Peter Anvin @ 2011-05-23 20:05 UTC (permalink / raw)
  To: John Kacur
  Cc: Steven Rostedt, Eduardo Silva, Frederic Weisbecker, Ingo Molnar,
	Thomas Gleixner, x86, linux-kernel

On 05/23/2011 01:01 PM, John Kacur wrote:
> On Mon, 23 May 2011, Steven Rostedt wrote:
>> The real solution is to change all the users into const *. This should
>> be doable as the new and old pointers are not modified.


> 
> From 71d6e5971cc90a03fd2d7bf8aaa2b81932bd0c7c Mon Sep 17 00:00:00 2001
> From: John Kacur <jkacur@redhat.com>
> Date: Mon, 23 May 2011 21:24:27 +0200
> Subject: [PATCH] ftrace: Change unsigned char *new, *old to const
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> This removes the warning:
> arch/x86/kernel/ftrace.c: In function ‘ftrace_make_nop’:
> /arch/x86/kernel/ftrace.c:308: warning: assignment discards qualifiers from pointer target type
> 
> Steven Rostedt suggested to make these pointers const pointers.
> 
> This patch assumes that
> https://patchwork.kernel.org/patch/798172/
> is applied first.
> 
> Reported-by: Eduardo Silva <edsiper@gmail.com>
> Signed-off-by: John Kacur <jkacur@redhat.com>

Looks much better.

Acked-by: H. Peter Anvin <hpa@zytor.com>

	-hpa

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-05-23 20:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-23  1:19 [PATCH] kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace() Eduardo Silva
2011-05-23  1:36 ` H. Peter Anvin
2011-05-23  2:27   ` Eduardo Silva
2011-05-23 13:08   ` Steven Rostedt
2011-05-23 20:01     ` John Kacur
2011-05-23 20:05       ` H. Peter Anvin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).