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 <[email protected]>
---
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
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 <[email protected]>
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.
On Sun, May 22, 2011 at 9:36 PM, H. Peter Anvin <[email protected]> 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 <[email protected]>
>
> 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
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 <[email protected]>
>
> 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
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 <[email protected]>
> >
> > 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 <[email protected]>
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 <[email protected]>
Signed-off-by: John Kacur <[email protected]>
---
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
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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: John Kacur <[email protected]>
Looks much better.
Acked-by: H. Peter Anvin <[email protected]>
-hpa