[Lustre-discuss] Finding bugs in Lustre with Coccinelle

[Andreas Dilger]

Isaac,
I'm all in favor of using static code analysis tools to find bugs like this. The first step, as you have done is to find and fix the bugs (though with proper patches since LASSERT() as a means of error handling is unacceptable). 

After this it would be possible to add such a script to the build and/or prepare-commit-msg hook so that it detects such errors early in the development process. 

We are now entering a 3 month feature freeze for the 2.2 branch, so it is a perfect time to fix latent bugs like this before the 2.2 release. Please don't delay too long since the last month will be a code freeze and only bugs being hit during testing will be landed. 

Cheers, Andreas

On 2012-01-08, at 0:58, Isaac Huang <isaac_huang at xyratex.com> wrote:

> Today I decided to try Coccinelle on latest Lustre code found on
> master at git://git.whamcloud.com/fs/lustre-release.git.
> 
> I came up with a simple Coccinelle script that tries to detect the
> case where a new object is allocated and dereferenced without checking
> it against NULL.
> 
> Eight such bugs were unconvered:
> $ spatch -sp_file /tmp/LIBCFS_ALLOC.cocci -dir lnet 2>/dev/null > /tmp/lustre.diff
> $ spatch -sp_file /tmp/OBD_ALLOC.cocci -dir lustre 2>/dev/null >> /tmp/lustre.diff
> $ diffstat /tmp/lustre.diff
> b/llite/dir.c          |    2 ++
> b/mdc/lproc_mdc.c      |    1 +
> b/mgc/mgc_request.c    |    1 +
> b/obdclass/obd_mount.c |    2 ++
> b/selftest/conctl.c    |    1 +
> obdclass/obd_mount.c   |    1 +
> 6 files changed, 8 insertions(+)
> 
> I've attached here the lustre.diff, which contained dummy fixes. I
> hope they do get fixed. OBD_ALLOC.cocci is also attached.
> LIBCFS_ALLOC.cocci is almost identical - I guess they could be
> combined into one with some regex matching but I haven't figured out
> how yet.
> 
> This is to demonstrate how useful such a tool can be. The Linux kernel
> seems to have already integrated Coccinelle checks in the build
> system. Lustre could adopt something similar, and Coccinelle could do
> much more complex things than this.
> 
> - Isaac
> <OBD_ALLOC.cocci>
> <lustre.diff>
> _______________________________________________
> Lustre-discuss mailing list
> Lustre-discuss at lists.lustre.org
> http://lists.lustre.org/mailman/listinfo/lustre-discuss