Hi,
On 06/05/2021 13:19, Schneider, Robert wrote:
> Hi,
>
> What are the coding and style guidelines when you want to propose a patch to cryptsetup?
Code style is very similar to Linux kernel code style.
I will add some doc later (also with "how to contribute docs") etc.
>
> I don't quite get a clear picture from the source code for some things, and I couldn't find documentation on it inside the repo.
> I can write a clang-format ruleset if I know the rules...
If a formatting exception is more readable, then just format the code differently.
IMO source code is for people, not for machines enforcing rulesets.
(This ends with dealing with patches reshuffling spaces only, we do not need that.)
Anyway, if you plan to write something more sophisticated, better report issue
and discuss it first, please.
Some notes below, but these are not set in stone.
> Here are some things I've noticed while reading the source code, and some remaining questions:
>
> - mostly restricted to C90, e.g. variable declarations at top of functions; but there some use of C99 features like designators in array initializations
> -> do we have to declare variables at the top of functions?
yes
> -> are // style comments allowed?
Just for temporary comments (like TODO etc). Definitely prefer C /**/ comments.
> - would adding -pedantic in GCC be acceptable? (not sure how that would work in autotools)
Not for now. I use many extra warnings for tests, but pedantic is producing really garbage sometimes.
Better spent time with running static analyzers (scan-build, coverity, LGTM, ...)
Both gcc a clang compilation should be clean.
> - tabs are used for indentation
yes, tabs everywhere, even in test scripts.
> - unclear if spaces for alignment are OK
> -> tab == 8 spaces?
as kernel - default is 8
> - how to indent when wrapping lines, e.g. for function parameters? 2 tabs? Align at ( with spaces?
> -> sometimes, function declarations put every parameter on its own line
This is not strict, just keep it readable (same as other in the same file).
This (and comments) is the only part where tabs and spaces are mixed.
Definitely it is not unified now.
> - top-level declarations like functions and structs have { on new line, if/loops/etc inside functions have { on same line, else on same line as }, if/else without braces is allowed
> - one space between if/while/for/.. and the (> - documentation of API via doxygen (@ style, not \ style)
> - internal documentation via plain comments, no text on first line and all lines starting with aligned *
All above - yes, very similar to kernel codestyle.
> - max line length?
Lively discussion on this topic is in progress :-)
I would say: if the code is readable, prefer old-school 80 chars.
If longer line helps, no problem but try to exceed 100-110 chars.
Do not reformat embedded foreign code (currently mainly Argon2 crypto lib).
> - the * for pointers binds to the variable name (void *p instead of void* p)
> - multiple declarations in the same statement are allowed
yes
> - functions with external linkage have a prefix like LUKS2_
LUKS2 is internal prefix (for sub-format dirs, similar to LUKS/TCRYPT,VERITY, ...),
Libcryptsetup uses crypt_* prefix for exported objects.
> - types are not typedef'ed -> struct crypt_device instead of crypt_device_t
not everywhere, something was based on API evolution and it makes no sense to change
it just for the typedef cleanups.
> - there are file comments with copyright notices
> -> how to create a new file?
Copyright note should always mention authors of that file.
(Otherwise it no longer makes much sense, but we keep that for tracking authors.)
I try to license new code with LGPL 2.1 or any later, but most of code is still GPL2 or any later.
(Cannot fix it globally, there was not agreement of former authors to switch to LGPL.)
Adding file - just copy header with proper license (GPL or LGPL only as above) and copyright.
There are some internal headers with function prototypes, use these.
But why do you need to add source code files?
Milan
_______________________________________________
dm-crypt mailing list -- dm-crypt@xxxxxxxx
To unsubscribe send an email to dm-crypt-leave@xxxxxxxx
