| Message ID | 15baf76c271a0ae09f7b8556e50f2b4251e7049d.1674617130.git.jpoimboe@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | bc2c6f5695ffa05c838b8b6fc5cd581a672151a1 |
| Headers | show |
| Series | powerpc: Fix livepatch module re-patching issue | expand |
On Tue, Jan 24, 2023 at 7:38 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > restore_r2() returns 1 on success, which is surprising for a non-boolean > function. Change it to return 0 on success and -errno on error to match > kernel coding convention. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Acked-by: Song Liu <song@kernel.org>
On Tue 2023-01-24 19:38:04, Josh Poimboeuf wrote: > restore_r2() returns 1 on success, which is surprising for a non-boolean > function. Change it to return 0 on success and -errno on error to match > kernel coding convention. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Looks good: Reviewed-by: Petr Mladek <pmladek@suse.com> It is in the right direction. Just note that there are more functions with the boolean semantic passed via int return value. But there are also other functions already using the 0/-E* return values so this is rather positive change. Best Regards, Petr
On Tue, 24 Jan 2023, Josh Poimboeuf wrote: > restore_r2() returns 1 on success, which is surprising for a non-boolean > function. Change it to return 0 on success and -errno on error to match > kernel coding convention. > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> Reviewed-by: Miroslav Benes <mbenes@suse.cz> M
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c index 1096d6b3a62c..016e79bba531 100644 --- a/arch/powerpc/kernel/module_64.c +++ b/arch/powerpc/kernel/module_64.c @@ -504,7 +504,7 @@ static int restore_r2(const char *name, u32 *instruction, struct module *me) u32 *prev_insn = instruction - 1; if (is_mprofile_ftrace_call(name)) - return 1; + return 0; /* * Make sure the branch isn't a sibling call. Sibling calls aren't @@ -512,19 +512,16 @@ static int restore_r2(const char *name, u32 *instruction, struct module *me) * restore afterwards. */ if (!instr_is_relative_link_branch(ppc_inst(*prev_insn))) - return 1; + return 0; if (*instruction != PPC_RAW_NOP()) { pr_err("%s: Expected nop after call, got %08x at %pS\n", me->name, *instruction, instruction); - return 0; + return -ENOEXEC; } /* ld r2,R2_STACK_OFFSET(r1) */ - if (patch_instruction(instruction, ppc_inst(PPC_INST_LD_TOC))) - return 0; - - return 1; + return patch_instruction(instruction, ppc_inst(PPC_INST_LD_TOC)); } int apply_relocate_add(Elf64_Shdr *sechdrs, @@ -648,8 +645,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, strtab + sym->st_name); if (!value) return -ENOENT; - if (!restore_r2(strtab + sym->st_name, - (u32 *)location + 1, me)) + if (restore_r2(strtab + sym->st_name, + (u32 *)location + 1, me)) return -ENOEXEC; } else value += local_entry_offset(sym);
restore_r2() returns 1 on success, which is surprising for a non-boolean function. Change it to return 0 on success and -errno on error to match kernel coding convention. Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> --- arch/powerpc/kernel/module_64.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)