On Mon 2022-02-28 11:46:33, Christophe Leroy wrote:

Le 28/02/2022 à 11:56, Petr Mladek a écrit :

quoted

On Fri 2022-02-25 16:49:31, Christophe Leroy wrote:

quoted

Le 25/02/2022 à 10:34, Petr Mladek a écrit :

quoted

Please do not do these small coding style changes. It complicates the
review and increases the risk of regressions. Different people
have different preferences. Just imagine that every half a year
someone update style of a code by his personal preferences. The
real changes will then get lost in a lot of noise.

I disagree here. We are not talking about people's preference here but
compliance with documented Linux kernel Codying Style and handling of
official checkpatch.pl script reports.

Really?

1. I restored

	+	if (mod->klp_info->secstrings == NULL) {

    and checkpatch.pl is happy.

On mainline's kernel/module.c checkpatch.pl tells me:

CHECK: Comparison to NULL could be written "!mod->klp_info->secstrings"
#2092: FILE: kernel/module.c:2092:
+	if (mod->klp_info->secstrings == NULL) {

Only with --strict option. Alias of this option is --subjective...

By the way some maintainers require checkpatch' clean patches even when 
this is only code move. I remember being requested to do that in the 
past, so now I almost always do it with my own patches.

I see.

From my POV, checkpatch is an useful tool for finding obvious mistakes.
But it is just a best effort approach. It has false positives. And
some complains are controversial.

BTW: I have never heard about --strict/--subjective option. I wonder
if some maintainer requires it.

quoted

I would not have complained if it did not complicate my review.
But it did!

Reviewing partial code move is not easy anyway, git is not very 
userfriendly with that.

Exactly. It is a real pain to find changes in moved functions. It is
much easier when the author just shuffled the code. Anyway, the less
changes the better.

Best Regards,
Petr