Re: [PATCH 2/2] MIPS: kprobes: Massage previous delay slot detection

[Maciej W. Rozycki]

On Sun, 8 Sep 2024, Jiaxun Yang wrote:

> Expand the if condition into cascaded ifs to make code
> readable.

 Apart from broken formatting what's making original code unreadable?

> Also use sizeof(union mips_instruction) instead of
> sizeof(mips_instruction) to match the code context.

 That has to be a separate change.

> diff --git a/arch/mips/kernel/kprobes.c b/arch/mips/kernel/kprobes.c
> index dc39f5b3fb83..96139adefad2 100644
> --- a/arch/mips/kernel/kprobes.c
> +++ b/arch/mips/kernel/kprobes.c
> @@ -89,12 +89,12 @@ int arch_prepare_kprobe(struct kprobe *p)
>  		goto out;
>  	}
>  
> -	if (copy_from_kernel_nofault(&prev_insn, p->addr - 1,
> -			sizeof(mips_instruction)) == 0 &&
> -	    insn_has_delayslot(prev_insn)) {
> -		pr_notice("Kprobes for branch delayslot are not supported\n");
> -		ret = -EINVAL;
> -		goto out;
> +	if (!copy_from_kernel_nofault(&prev_insn, p->addr - 1, sizeof(union mips_instruction))) {

 Overlong line.

> +		if (insn_has_delayslot(prev_insn)) {
> +			pr_notice("Kprobes for branch delayslot are not supported\n");

 This now overruns 80 columns making code *less* readable.

  Maciej