OCFS2 Coding Style
We follow the Linux Kernel Coding Style for both the modules and the tools. This is not optional. As OCFS2 is part of the mainline kernel, we have to ensure that all our code follows this generally accepted style.
Making a Patch
The following are a set of guidelines that apply towards making any patch against any tree.
Patches should be in the diff -u -p format with a "-p1" strip level. SVN patches are an exception since SVN makes that hard.
- Each patch must be a logical unit. As in, if a patch fixes a particular bug, it must contain changes pertaining only for that fix. We do this because it makes it easier to track changes. If you have a set of patches which build upon each other, you can send them in a series, but with each change within it's own patch.
- Patches must include a descriptive comment at the top which will be committed in with the change. The format we are using for comments is one line on it's own starting with "ocfs2: " and giving a short description, a short paragraph below with more details (if need be), and then the proper sign-off lines. "Signed-off-by:" means different things for GIT and SVN and will be explained in their respective sections. An example of an upstream commit is:
ocfs2: move nlink check in ocfs2_mknod() The dir nlink check in ocfs2_mknod() was being done outside of the cluster lock, which means we could have been checking against a stale version of the inode. Fix this by doing the check after the cluster lock instead. Signed-off-by: Mark Fasheh <mark.fasheh@oracle.com>
- Patches must be tested before submission. When changing code in the libraries, developer must determine the tools affected by the change and test all those tools. Also, the developer must get clearance from the tool owner.
Patches should preferably be emailed to the proper mailing lists. Private patch review (before sending to a public list) is always fine. For tools patches, this is ocfs2-tools-devel@oss.oracle.com. For OCFS2 patches, this is ocfs2-devel@oss.oracle.com. For test suite patches, this is ocfs2-test@oss.oracle.com. We do this because it allows a wider audience to provide code review. The entire OCFS2 stack is an open source project, and we must treat it as such.
GIT Patches
OCFS2 is in the mainline kernel, and the majority of file system patches will be against what is called "the git tree". Instructions and information on how git works with OCFS2 can be found at GitForOCFS2
The source control page also has a good overview of the structure of ocfs2.git: http://oss.oracle.com/projects/ocfs2/source.html
Also, please follow the kernel guidelines. As these patches will invariably be posted upstream, it is important we follow these guidelines.
SVN Patches
For patches against the svn tree, please follow the following:
By default svn diff does not create patches in the diff -u -p format. Steps for configuring the same as listed below.
SVN patches should be in the patch -p0 form.
Index: tunefs.ocfs2/tunefs.c =================================================================== --- tunefs.ocfs2/tunefs.c.orig 2006-07-18 14:31:39.927014000 -0700 +++ tunefs.ocfs2/tunefs.c 2006-07-18 14:35:19.133806000 -0700
Patches must be committed with the "Signed-off-by: <id>" tag to indicate all the reviewers.
- For subversion patches, "Signed-off-by: " means something slightly different than for mainline. It is used as a mechanism to show who reviewed a patch and determined that it was ok for commit. You only need one "sob" line, but multiple can be provided.
Configuring SVN diff
By default svn diff does not show the function name where the change has occurred. This makes reviewing patches a bit tedious as one has to apply the patch to make sense of it.
To make svn diff show the function names, do as follows:
- Create a svn diff wrapper.
$ cat ~/bin/svn_diff_wrapper #!/bin/bash exec /usr/bin/diff -u -p "$@"
- Edit ~/.subversion/config to reference the wrapper.
[helpers] diff-cmd = <path>/svn_diff_wrapper