Skip to contentSkip to conversation

x86, ftrace: Fix compiler warning in ftrace.c

PATCH In review Rakib Mullick 2 participants 4 messages May 12, 2011 – May 18, 2011 2 messages not in the archive

Subsystems (2)
x86 ftrace
lore
Share & tools
lore

Rakib Mullick

author
#0 lore raw

PATCH 1/1 x86, ftrace: Fix compiler warning in ftrace.c

Due to commit dc326fca2b64 (x86, cpu: Clean up and unify the NOP selection infrastructure), we get the following warning:

arch/x86/kernel/ftrace.c: In function ‘ftrace_make_nop’:
arch/x86/kernel/ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type arch/x86/kernel/ftrace.c: In function ‘ftrace_make_call’:
arch/x86/kernel/ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type

ftrace_nop_replace() now returns const unsigned char *, so change its associated function/variable to its compatible type to keep compiler clam.

Signed-off-by Rakib Mullick <[email protected]>

---

arch/x86/kernel/ftrace.c +5 -5
index 0ba15a6..d2d82cf 100644
@@ -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, unsigned const char *old_code,
+		   unsigned const char *new_code)
 {
 	unsigned char replaced[MCOUNT_INSN_SIZE];
 

@@ -290,7 +290,7 @@ ftrace_modify_code(unsigned long ip, unsigned char *old_code,
 		return -EINVAL;
 
 	/* replace the text with the new text */
-	if (do_ftrace_mod_code(ip, new_code))
+	if (do_ftrace_mod_code(ip, (void *)new_code))
 		return -EPERM;
 
 	sync_core();

@@ -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;
+	unsigned const 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;
+	unsigned const char *new, *old;
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace();
maintainer
#1 lore raw
Quoted 29 lines · Rakib Mullick

Due to commit dc326fca2b64 (x86, cpu: Clean up and unify the NOP selection infrastructure), we get the following warning:

arch/x86/kernel/ftrace.c: In function ‘ftrace_make_nop’:
arch/x86/kernel/ftrace.c:308:6: warning: assignment discards qualifiers from pointer target type arch/x86/kernel/ftrace.c: In function ‘ftrace_make_call’:
arch/x86/kernel/ftrace.c:318:6: warning: assignment discards qualifiers from pointer target type

ftrace_nop_replace() now returns const unsigned char *, so change its associated function/variable to its compatible type to keep compiler clam.

Signed-off-by Rakib Mullick <[email protected]>

---

arch/x86/kernel/ftrace.c +2 -2
index 0ba15a6..d2d82cf 100644
@@ -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, unsigned const char *old_code,
+		   unsigned const char *new_code)
 {
 	unsigned char replaced[MCOUNT_INSN_SIZE];

arch/x86/kernel/ftrace.c ftrace_modify_code(unsigned long ip, unsigned char *old_code, · line 293

Quoted 5 lines
arch/x86/kernel/ftrace.c +1 -1
@@ -290,7 +290,7 @@ ftrace_modify_code(unsigned long ip, unsigned char *old_code,
 		return -EINVAL;
 
 	/* replace the text with the new text */
-	if (do_ftrace_mod_code(ip, new_code))
+	if (do_ftrace_mod_code(ip, (void *)new_code))

Instead of typecasting, why not make new_code const void *?

This would probably trickle down to change __probe_kernel_write() which would not be a bad thing.

-- Steve

Quoted 22 lines
arch/x86/kernel/ftrace.c +2 -2
 		return -EPERM;
 
 	sync_core();

@@ -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;
+	unsigned const 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;
+	unsigned const char *new, *old;
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace();

A message that is not in the archive (sent off-list or not yet received): [email protected]

maintainer
#3 lore raw
Quoted 1 line · Rakib Mullick

BTW, You have a weird email client. Causes lots of weird whitespace

Quoted 2 lines

There are other places where they depend on __probe_kernel_write(), it might require to change all __probe_kernel_write() callers.

Why would callers of __probe_kernel_write() need to be changed? You can pass any pointer into a const void*. You just can't do it the other way around.

-- Steve

Quoted 2 lines

A message that is not in the archive (sent off-list or not yet received): [email protected]

maintainer
#5 lore raw
Quoted 8 lines · Rakib Mullick

Cause, __probe_kernel_write() currently takes void * as its parameter. That means all the users of __probe_kernel_write() currently passes void *. If we change __probe_kernel_write's parameter ( src) from void
* to const void * , then all the callers also needs to do that. Isn't it?

No.

Look at __probe_kernel_write() itself. It calls __copy_to_user_in_atomic() which takes const void *src, where we pass in void *src. That __probe_kernel_write() having void *src is actually a bug.

Make that a separate patch, or maybe I'll just do it.

-- Steve