Skip to contentSkip to conversation

kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()

PATCH In review Eduardo Silva 4 participants 6 messages May 23, 2011

Subsystems (2)
x86 ftrace
lore
Share & tools
lore

Eduardo Silva

author
#0 lore raw

PATCH 1/1 kernel: Fix ftrace.c compiler warning when calling ftrace_nop_replace()

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(-)
arch/x86/kernel/ftrace.c +2 -2
index 0ba15a6..b44c3c9 100644
@@ -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);
Signature · 1 line
1.7.4.1
reviewer
#1 lore raw
Quoted 10 lines · Eduardo Silva

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

Signature · 2 lines
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.
author
#2 lore raw
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · Eduardo Silva

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

Quoted 6 lines
Quoted 3 lines

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,

Quoted 9 lines

       -hpa

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

Signature · 3 lines
Eduardo Silva
http://edsiper.linuxchile.cl
http://www.monkey-project.com
maintainer
#3 lore raw
In reply to H. Peter Anvin
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · Eduardo Silva

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

Quoted 6 lines
Quoted 3 lines

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

#4 lore raw
Quoted 24 lines · Steven Rostedt
Quoted 8 lines · H. Peter Anvin
Quoted 7 lines · Eduardo Silva

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

Quoted 6 lines
Quoted 3 lines

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:

Quoted 8 lines
	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.

Quoted 1 line

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(-)
arch/x86/kernel/ftrace.c +6 -6
index 0ba15a6..e4f23d1 100644
@@ -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();
Signature · 1 line
1.7.2.3
reviewer Acked-by
#5 lore raw
Quoted 18 lines · John Kacur
Quoted 2 lines · Steven Rostedt

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

Quoted 8 lines

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