在2024年9月9日九月 下午11:02，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?
For me it's confusing because wired layout, cascaded ifs are clearly
easier to format and has clear intention.
>
>> Also use sizeof(union mips_instruction) instead of
>> sizeof(mips_instruction) to match the code context.
>
> That has to be a separate change.
Given that it's a tiny style change as well, it makes sense to combine
into same patch.
>
>> 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.
Nowadays, check-patch.pl is happy with 100 column line.
I used 100 column line in many subsystems and never receive any complaint.
>
>> + 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.
I don't really agree, we are not in VGA display era any more, see Linus's
arguments on removal of 80 columns [1] and why long line are more readable [2].
[1]: https://lore.kernel.org/lkml/CAHk-=wj3iGQqjpvc+gf6+C29Jo4COj6OQQFzdY0h5qvYKTdCow@xxxxxxxxxxxxxx/
[2]: https://lore.kernel.org/lkml/CAHk-=wjR0H3+2ba0UUWwoYzYBH0GX9yTf5dj2MZyo0xvyzvJnA@xxxxxxxxxxxxxx/
Thanks
>
> Maciej
--
- Jiaxun
