Skip to content

lzfse/: cleanup files #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

ethancedwards8
Copy link

To be merged into the mainline kernel, these library files need to be cleaned up. Correcting the license headers and formatting the code with clang-format is a good start.

Additionally, some of the C files are not used/compiled into the final module. I understand that you have probably kept them around for future implementations, however, since they are not used, should they just be removed? Especially the lzfse_main.c file?

Linux kernel coding styles require SPDX headers at the top of each file.

Signed-off-by: Ethan Carter Edwards <[email protected]>
clang-format is used in the mainline to format files. Additionally,
there are occasional #if's used to see if the windows compiler is used.
Obviously, the windows compiler will not be used to compile the kernel.
We can safely remove them.

Signed-off-by: Ethan Carter Edwards <[email protected]>
@eafer
Copy link
Member

eafer commented Mar 17, 2025

I haven't reviewed this yet, but keep in mind that the kernel has its own style checker in tree, it's a script called checkpatch.pl. You should probably use that instead of clang-format.

@ethancedwards8
Copy link
Author

There is an in-tree kernel configuring file for .clang-format which follos kernel styles. https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/.clang-format .

I am aware of checkpatch.pl, but clang-format is pretty good I think.

@eafer
Copy link
Member

eafer commented Mar 17, 2025

Ah, I didn't know about that. Then I guess it's fine.

@eafer
Copy link
Member

eafer commented Mar 26, 2025

The comment style didn't get fixed, in fact it got broken in a new way in some places. Weird, I guess clang-format doesn't cover everything.

@ethancedwards8
Copy link
Author

Ah. Good to know. I'll fix soon, thanks.

@eafer
Copy link
Member

eafer commented Apr 12, 2025

I've been cleaning this up myself, so I think I can close this now. Thanks anyway.

@eafer eafer closed this Apr 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants