Re: [PATCH 3/3] powerpc/kprobes: Check return value of patch_instruction()

[Steven Rostedt <rostedt@goodmis.org>]

On Sat, 25 Apr 2020 10:11:56 +0000
Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> 
> Sure it's be more explicit, but then more lines also. 3 lines for only 
> one really usefull.
> 
> With goto, I would look like:
> 
> diff --git a/arch/powerpc/kernel/optprobes.c 
> b/arch/powerpc/kernel/optprobes.c
> index 046485bb0a52..938208f824da 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -139,14 +139,14 @@ void arch_remove_optimized_kprobe(struct 
> optimized_kprobe *op)
>   	}
>   }
> 
> -#define PATCH_INSN(addr, instr)						     \
> +#define PATCH_INSN(addr, instr, label)						     \

With the explicit label as a parameter, makes it more evident that it
will do something (like jump) with that label.

I like this solution the best!

-- Steve


>   do {									     \
>   	int rc = patch_instruction((unsigned int *)(addr), instr);	     \
>   	if (rc) {							     \
>   		pr_err("%s:%d Error patching instruction at 0x%pK (%pS): %d\n", \
>   				__func__, __LINE__,			     \
>   				(void *)(addr), (void *)(addr), rc);	     \
> -		return rc;						     \
> +		goto label;						     \
>   	}								     \
>   } while (0)
>