[plbossart]

It seems that everyone has dropped the ball on following coding guidelines, or no one cares about cleaning things up.

Before adding silly new rules to checkpatch, maybe we should first enforce its use and make sure the existing code is consistent. Here's a first batch I did during long debug sessions on the Linux side, there's more to come. I started from the master branch, did a reset to the origin and re-added all the files in separate patches, so checkpatch looked at all the history, not just the last contributions.

What I really looked for is:

• misaligned parameters and conditions
• multiple assignments
• macros
• spaces between casts/operands
• balanced brackets
• !pointer instead of pointer == NULL
• block comments with ending at end of line instead of newline
• logical statements on previous line rather than newline
• spelling. dont/cant/postion/her, etc.

Disclaimer: no tests were run, this is just based on checkpatch hints and checks are needed.

[RanderWang]

here, should be

/*
 * xxx
 * xxx
 */
?

[singalsu]

LGTM and thanks for fixing my indents!

[lgirdwood]

It seems that everyone has dropped the ball on following coding guidelines, or no one cares about cleaning things up.

Checkpatch should be an automatic git hook now (for all users) so most of the fixes should have been for style issues that pre-dated it's introduction. Although some may have slipped through if authors disabled the hook or committed with -n :(

@plbossart you could quickly validate by building all images with and without this patch with debug disabled and comparing binaries (they should be the same)

@mengdonglin can someone validate. thanks

[jajanusz]

IMHO formatting shouldn't be human work.
We should have something like:
https://github.com/torvalds/linux/blob/master/.clang-format

Run it on all files to fix existing codebase then make hook for checks (it still can work with checkpatch, it's not meant to be a replacement).

This way everyone would be able to format file with just a key shortcut.

If you really would like to have some weird things like ascii art in comments you can always make it work even with clang-format

[singalsu]

Emacs can be configured to on-the-fly during editing format code in such way that passes the SOF check script. Though I use mainly Netbeans for development editing and there the Linux kernel code style misses some things. So, if I get fail from pre/post commit check I fix the issue with Emacs.

Though our new long traces always violate the the checker. Can the the checker be fixed to handle them?

[xiulipan]

@singalsu I think @plbossart have done the function name warn for trace here.

[singalsu]

@xiuli Great that the annoyance is fixed! Due to that I've unfortunately had most of the time the pre commit script disabled.

[plbossart]

On 1/16/19 6:48 AM, Janusz Jankowski wrote: IMHO formatting shouldn't be human work. We should have something like: https://github.com/torvalds/linux/blob/master/.clang-format Run it on all files to fix existing codebase then make hook for checks (it still can work with checkpatch, it's not meant to be a replacement). This way everyone would be able to format file with just a key shortcut. If you really would like to have some weird things like ascii art in comments you can always make it work even with clang-format

Yes and no. A tool does not read code the way humans do. Sometimes it's better to violate some rules to keep the code readable and there are many cases where I left the code as is. I am not advocating for absolute compliance with the rules, but that the tool be used. For the Linux side, I always look at the issues reported by checkpatch, if any, and will decide if the report needs to be fixed or not.