Lustre Coding Style Guidelines - Revision history

Adilger: /* Enumerated Type Declarations */ add comment to avoid 0 for enums if possible

2026-08-06T06:55:01Z

Enumerated Type Declarations: add comment to avoid 0 for enums if possible

← Older revision		Revision as of 23:55, 5 August 2026
Line 451:		Line 451:

	Enums that form part of the ABI or wire protocol (e.g. under <code>lustre/include/uapi</code> should always have explicit values assigned instead of automatically enumerated values. This ensures that the values remain constant in the protocol/ABI even if intermediate constants are added or removed.		Enums that form part of the ABI or wire protocol (e.g. under <code>lustre/include/uapi</code> should always have explicit values assigned instead of automatically enumerated values. This ensures that the values remain constant in the protocol/ABI even if intermediate constants are added or removed.

			It is strongly recommended that enum values only use <code>0</code> as an "uninitialized" or "unset" value, and start values at <code>1</code> (or as needed). This allows easier (default) initialization of a struct to zero, and detection if a struct is overwritten by zeroes (a typical corruption scenario). It also avoids issues where the <code>0</code> enum value is used as a bitmask, and incorrectly tries to set/check this "bit" in a flag, which is impossible if it is 0.

	==== Sequence Declarations ====		==== Sequence Declarations ====

Adilger: /* Enumerated Type Declarations */ discuss enum values

2026-01-16T01:19:08Z

Enumerated Type Declarations: discuss enum values

← Older revision		Revision as of 18:19, 15 January 2026
Line 449:		Line 449:

	Also, the compiler can do argument verification when a named <code>enum</code> is the type, and tools like <code>gdb</code> and <code>strace</code> can interpret numeric arguments into the named values.		Also, the compiler can do argument verification when a named <code>enum</code> is the type, and tools like <code>gdb</code> and <code>strace</code> can interpret numeric arguments into the named values.

			Enums that form part of the ABI or wire protocol (e.g. under <code>lustre/include/uapi</code> should always have explicit values assigned instead of automatically enumerated values. This ensures that the values remain constant in the protocol/ABI even if intermediate constants are added or removed.

	==== Sequence Declarations ====		==== Sequence Declarations ====

Adilger: /* Style and Formatting Guidelines */ remove old checkpatch.pl reference

2026-01-16T01:17:16Z

Style and Formatting Guidelines: remove old checkpatch.pl reference

← Older revision		Revision as of 18:17, 15 January 2026
Line 44:		Line 44:

	== Style and Formatting Guidelines ==		== Style and Formatting Guidelines ==
	All of our rules for formatting, wrapping, parenthesis, brace placement, etc., are derived from the Linux kernel rules, which are basically K&R style. Some of these rules are automatically verified at commit time by the <code>contrib/scripts/checkpatch.pl</code> script ~~(formerly <code>build/checkpatch.pl</code>)~~ included with newer versions of the Lustre code, but many depend on the good judgment of the coder and inspector. Note that there is also [[Lustre Script Coding Style]] that describes the formatting for shell scripts used for userspace utilities and testing.		All of our rules for formatting, wrapping, parenthesis, brace placement, etc., are derived from the Linux kernel rules, which are basically K&R style. Some of these rules are automatically verified at commit time by the <code>contrib/scripts/checkpatch.pl</code> script included with newer versions of the Lustre code, but many depend on the good judgment of the coder and inspector. Note that there is also [[Lustre Script Coding Style]] that describes the formatting for shell scripts used for userspace utilities and testing.

	=== Git Checkpatch Hooks ===		=== Git Checkpatch Hooks ===
Line 51:		Line 51:
	$ ln -sf ../../contrib/git-hooks/*commit-msg .git/hooks/		$ ln -sf ../../contrib/git-hooks/*commit-msg .git/hooks/

	At <code>git commit</code> time this will run the <code>.git/hooks/prepare-commit-msg</code> script from the currently Lustre tree to check the code changes for style errors using <code>contrib/checkpatch.pl</code>, and add comments at the end of the commit message with any warnings or errors. After the commit message has been saved, the <code>.git/hooks/commit-msg</code> script will verify that the commit message format matches the format specified above. While the <code>checkpatch.pl</code> script is not perfect, most of the warnings are valid and should be addressed. Using symlinks instead of copying the hooks will ensure that the hooks are updated appropriately between branches and as changes are made to the scripts.		At <code>git commit</code> time this will run the <code>.git/hooks/prepare-commit-msg</code> script from the currently Lustre tree to check the code changes for style errors using <code>contrib/checkpatch.pl</code>, and add comments at the end of the commit message with any warnings or errors. After the commit message has been saved, the <code>.git/hooks/commit-msg</code> script will verify that the commit message format matches the format specified above. While the <code>checkpatch.pl</code> script is not perfect, 'most' of the warnings are valid and should be addressed. Using symlinks instead of copying the hooks will ensure that the hooks are updated appropriately between branches and as changes are made to the scripts.

	=== Whitespace and Comments ===		=== Whitespace and Comments ===

Adilger: /* Enumerated Type Declarations, Sequence Declarations */

2025-08-20T05:46:18Z

Enumerated Type Declarations, Sequence Declarations

@@ Line 444: / Line 444: @@
          return ret;
 </nowiki>
 ==== Wrapping Long Lines ====

Adilger: /* Memory Allocation */

2025-08-20T05:30:45Z

Memory Allocation

← Older revision		Revision as of 22:30, 19 August 2025
Line 730:		Line 730:
	OBD_ALLOC_PTR(mds_body);		OBD_ALLOC_PTR(mds_body);
	OBD_FREE_PTR(mds_body);		OBD_FREE_PTR(mds_body);

			OBD_ALLOC_PTR_ARRAY(array, 10);
			OBD_FREE_PTR_ARRAY(array, 10);
	</nowiki>		</nowiki>
	'''wrong:'''<nowiki>		'''wrong:'''<nowiki>
Line 737:		Line 740:
	* When allocating a large variable (pretty much anything above 8KiB, use <code>OBD_ALLOC_LARGE</code> and <code>OBD_FREE_LARGE()</code> in order to avoid allocation failures. This will first try to use <code>kmalloc()</code> for the fastest allocation, but if this fails (or is too large for <code>kmalloc()</code>) then it will fall back to using <code>vmalloc()</code> as needed. This makes it easier to see which allocations may consume a lot of RAM, and avoids errors hit on long-running systems when memory is fragmented that are not seen during short-lived test runs or right after mount.		* When allocating a large variable (pretty much anything above 8KiB, use <code>OBD_ALLOC_LARGE</code> and <code>OBD_FREE_LARGE()</code> in order to avoid allocation failures. This will first try to use <code>kmalloc()</code> for the fastest allocation, but if this fails (or is too large for <code>kmalloc()</code>) then it will fall back to using <code>vmalloc()</code> as needed. This makes it easier to see which allocations may consume a lot of RAM, and avoids errors hit on long-running systems when memory is fragmented that are not seen during short-lived test runs or right after mount.
	* When allocating/freeing an array, use <code>OBD_ALLOC_ARRAY()</code> or <code>OBD_ALLOC_PTR_ARRAY()</code> or <code>OBD_ALLOC_ARRAY_LARGE()</code> and corresponding <code>FREE()</code> function to avoid overflow during calculation of array size.		* When allocating/freeing an array, use <code>OBD_ALLOC_ARRAY()</code> or <code>OBD_ALLOC_PTR_ARRAY()</code> or <code>OBD_ALLOC_ARRAY_LARGE()</code> and corresponding <code>FREE()</code> function to avoid overflow during calculation of array size.

	==== Proper use of LASSERT() ====		==== Proper use of LASSERT() ====
	* Do not embed operations inside assertions. If assertions are disabled for performance reasons this code will not be executed.		* Do not embed operations inside assertions. If assertions are disabled for performance reasons this code will not be executed.

Adilger: /* Lustre Variable Types */

2025-08-20T05:29:40Z

Lustre Variable Types

← Older revision		Revision as of 22:29, 19 August 2025
Line 715:		Line 715:
	int *array;		int *array;

	~~OBD_ALLOC_PTR_ARRAY(array, 10); /* correctly does math, no need to worry */~~		OBD_ALLOC(array, 10 * sizeof(array)); / correct, but using OBD_ALLOC_PTR_ARRAY() is preferred */
	OBD_ALLOC(array, 10 * sizeof(array)); / ~~also~~ correct, but ~~less~~ preferred */
	rc = strncpy(buf, src, sizeof(buf));		rc = strncpy(buf, src, sizeof(buf));
	</nowiki>		</nowiki>

Adilger: /* Lustre Variable Types */ add OBD_ALLOC_PTR_ARRAY()

2025-08-20T05:28:43Z

Lustre Variable Types: add OBD_ALLOC_PTR_ARRAY()

← Older revision		Revision as of 22:28, 19 August 2025
Line 715:		Line 715:
	int *array;		int *array;

	OBD_ALLOC(array, 10 * sizeof(*array));		OBD_ALLOC_PTR_ARRAY(array, 10); /* correctly does math, no need to worry */
			OBD_ALLOC(array, 10 * sizeof(array)); / also correct, but less preferred */
	rc = strncpy(buf, src, sizeof(buf));		rc = strncpy(buf, src, sizeof(buf));
	</nowiki>		</nowiki>
Line 724:		Line 725:
	rc = strncpy(buf, src, sizeof(buf)); / This is the character size, not array size */		rc = strncpy(buf, src, sizeof(buf)); / This is the character size, not array size */
	</nowiki>		</nowiki>

	==== Memory Allocation ====		==== Memory Allocation ====
	* When allocating/freeing a single struct, use <code>OBD_ALLOC_PTR()</code> for clarity:		* When allocating/freeing a single struct, use <code>OBD_ALLOC_PTR()</code> for clarity:

Adilger: /* Wrapping Long Lines */ improve line wrap discussion

2025-05-07T16:28:31Z

Wrapping Long Lines: improve line wrap discussion

← Older revision		Revision as of 09:28, 7 May 2025
Line 446:		Line 446:

	==== Wrapping Long Lines ====		==== Wrapping Long Lines ====
	* Even for short conditionals, the operation should be on a separate line:		* Even for short conditionals, the operation should be on a separate line from the condition:
	'''right:'''<nowiki>		'''right:'''<nowiki>
	if (foo)		if (foo)
Line 454:		Line 454:
	if (foo) bar();		if (foo) bar();
	</nowiki>		</nowiki>
	* When you wrap a line containing parenthesis, start the continued line after the parenthesis so that the expression or argument is visually bracketed.		* When you wrap a line containing parenthesis, start the continued line after the parenthesis on the previous line '''related to the sub-expression that is being split''', so that the expression or argument is visually bracketed.
	'''right:'''<nowiki>		'''right:'''<nowiki>
	variable = do_something_complicated(long_argument, longer_argument,		variable = do_something_complicated(long_argument, longer_argument,
Line 473:		Line 473:
	last_argument);		last_argument);

	if (some_long_condition(arg1, arg2, arg3) < some_long_value &&		if (some_long_condition(arg1, arg2, arg3) < some_long_value
	another_long_condition(very_long_argument_name,		&& another_long_condition(very_long_argument_name,
	another_long_argument_name) >		> another_long_argument_name)
	second_long_value) {		second_long_value) {
	do_something();		do_something();
	...		...
	</nowiki>		</nowiki>
	* If you're wrapping an expression, put the operator at the end of the line. If there are no parentheses to which to align the start of the next line, just indent one more tab stop.		* If you're wrapping an expression, '''put the operator at the end of the line'''. If there are no parentheses to which to align the start of the next line, just indent one more tab stop.
			'''right:'''<nowiki>
	off = le32_to_cpu(fsd->fsd_client_start) +		off = le32_to_cpu(fsd->fsd_client_start) +
	cl_idx * le16_to_cpu(fsd->fsd_client_size);		cl_idx * le16_to_cpu(fsd->fsd_client_size);

			if (some_long_condition(with, some, arguments) &&
			another_long_condition(with, more, arguments) &&
			final_condition + another_value >
			LONG_CONSTANT_NAME_FOR_COMPARISON)
			</nowiki>
	* Binary and ternary (but not unary) operators should be separated from their arguments by one space.		* Binary and ternary (but not unary) operators should be separated from their arguments by one space.
	'''right:'''<nowiki>		'''right:'''<nowiki>
Line 490:		Line 496:
	d = (f > g) ? 0 : 1;		d = (f > g) ? 0 : 1;
	</nowiki>		</nowiki>

	==== Function Calls and Spaces ====		==== Function Calls and Spaces ====
	* Function calls should be nestled against the parentheses, the parentheses should crowd the arguments, and one space should appear after commas:		* Function calls should be nestled against the parentheses, the parentheses should crowd the arguments, and one space should appear after commas:

Adilger: /* Use Tabs Instead of Spaces */ explain long strings better

2025-05-07T16:19:04Z

Use Tabs Instead of Spaces: explain long strings better

← Older revision		Revision as of 09:19, 7 May 2025
Line 61:		Line 61:
	* Tabs are preferred to align variable names in '''structure and enum declarations''' and their descriptive comments. This may ''temporarily'' misalign them with other variable names in the structure that are using spaces for indentation, or consider fixing up the whole struct in the same patch if there aren't too many of them. In some cases (long variable names, long comments, nested unions) it isn't practical to use full tabs to align the fields, so spaces can be used for partial indentation.		* Tabs are preferred to align variable names in '''structure and enum declarations''' and their descriptive comments. This may ''temporarily'' misalign them with other variable names in the structure that are using spaces for indentation, or consider fixing up the whole struct in the same patch if there aren't too many of them. In some cases (long variable names, long comments, nested unions) it isn't practical to use full tabs to align the fields, so spaces can be used for partial indentation.
	* Only a '''single space''' should separate the variable type and variable name for '''local function declarations'''. This is to match upstream Linux kernel coding style.		* Only a '''single space''' should separate the variable type and variable name for '''local function declarations'''. This is to match upstream Linux kernel coding style.
	* ~~All~~ lines should wrap at 80 characters. This is to avoid the need to have extra-wide terminal windows for the few lines of code that exceed 80 columns, and avoids nesting functions and conditionals too deeply~~. Exceptions to this rule in the upstream kernel include long text strings~~. If it's getting too hard to wrap at 80 characters, you probably need to rearrange conditional order or ~~break it up~~ into ~~more functions~~.		* Most lines should wrap at 80 characters. This is to avoid the need to have extra-wide terminal windows for the few lines of code that exceed 80 columns, and avoids nesting functions and conditionals too deeply. If it's getting too hard to wrap at 80 characters, you probably need to rearrange conditional order or move some code into a helper function. The main exception to this rule is long text strings, which should be kept on a single line so that the string can be found via <code>grep</code> from the message printed on the console.
	'''right:'''<nowiki>		'''right:'''<nowiki>
	void func_helper(...)		void func_helper(...)
Line 77:		Line 77:
	}		}

	~~void~~ func(...)		int func(...)
	{		{
	long first;		long first;
	int i;		int rc;

	if (!cond1) {		if (!cond1) {
	CERROR("some error message that is long but kept on one line\n");		rc = -EINVAL
	return;		CERROR("%s: some error message that is long but kept on one line: rc = %d\n",
			obd_name, rc);
			return rc;
	}		}

Line 93:		Line 95:

	do_sth1_2;		do_sth1_2;

			return rc;
	}		}
	</nowiki>		</nowiki>

Mvef: /* Use Common Cleanup and Return for Error Handling */

2024-12-19T15:32:18Z

Use Common Cleanup and Return for Error Handling

← Older revision		Revision as of 08:32, 19 December 2024
Line 363:		Line 363:
	{		{
	foo = alloc_foo();		foo = alloc_foo();
	if (foo ~~== NULL~~)		if (!foo)
	RETURN(NULL);		RETURN(NULL);
	rc = setup_foo(foo);		rc = setup_foo(foo);
	if (rc ~~!= 0~~) {		if (rc) {
	free_foo(foo);		free_foo(foo);
	RETURN(ERR_PTR(rc));		RETURN(ERR_PTR(rc));