d binderman
2010-Jan-19 13:18 UTC
static analysis tool cppcheck meets the btrfs code - four issues
Hello there,
I just ran the sourceforge tool cppcheck over the source code of the
new Linux kernel 2.6.32.4
It said
1.
[./btrfs/free-space-cache.c:600]: (style) Redundant condition. It is safe to
deallocate a NULL pointer
The source code is
if (info->bitmap)
kfree(info->bitmap);
I agree with cppcheck. Suggest delete the if test.
2.
[./btrfs/free-space-cache.c:906]: (style) Redundant condition. It is safe to
deallocate a NULL pointer
Duplicate.
3.
[./btrfs/relocation.c:3278]: (error) Memory leak: cluster
The source code is
cluster = kzalloc(sizeof(*cluster), GFP_NOFS);
if (!cluster)
return -ENOMEM;
path = btrfs_alloc_path();
if (!path)
return -ENOMEM;
Suggest new code
cluster = kzalloc(sizeof(*cluster), GFP_NOFS);
if (!cluster)
return -ENOMEM;
path = btrfs_alloc_path();
if (!path)
{
kfree( cluster);
return -ENOMEM;
}
4.
[./btrfs/volumes.c:1257]: (error) Possible null pointer dereference: fs_devices
The source code is
while (fs_devices) {
if (fs_devices->seed == device->fs_devices)
break;
fs_devices = fs_devices->seed;
}
fs_devices->seed = device->fs_devices->seed;
So presumably fs_devices can never be NULL i.e. the while loop will
always find what it is looking for and so no code to test for NULL is
required.
Regards
David Binderman
_________________________________________________________________
Do you have a story that started on Hotmail? Tell us now
http://clk.atdmt.com/UKM/go/195013117/direct/01/--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Wenyi Liu
2010-Jan-19 15:31 UTC
Re: static analysis tool cppcheck meets the btrfs code - four issues
2010/1/19 d binderman <dcb314@hotmail.com>:> > > Hello there, > > I just ran the sourceforge tool cppcheck over the source code of the > new Linux kernel 2.6.32.4 > > It said > > 1. > > [./btrfs/free-space-cache.c:600]: (style) Redundant condition. It is safe to deallocate a NULL pointer > > The source code is > > if (info->bitmap) > kfree(info->bitmap); > > I agree with cppcheck. Suggest delete the if test. > > 2. > > [./btrfs/free-space-cache.c:906]: (style) Redundant condition. It is safe to deallocate a NULL pointer > > Duplicate. > 3. > > [./btrfs/relocation.c:3278]: (error) Memory leak: cluster > > The source code is > > cluster = kzalloc(sizeof(*cluster), GFP_NOFS); > if (!cluster) > return -ENOMEM; > > path = btrfs_alloc_path(); > if (!path) > return -ENOMEM; > > Suggest new code > > cluster = kzalloc(sizeof(*cluster), GFP_NOFS); > if (!cluster) > return -ENOMEM; > > path = btrfs_alloc_path(); > if (!path) > { > kfree( cluster); > return -ENOMEM; > }sounds good, but the code format...> > 4. > > [./btrfs/volumes.c:1257]: (error) Possible null pointer dereference: fs_devices > > The source code is > > while (fs_devices) { > if (fs_devices->seed == device->fs_devices) > break; > fs_devices = fs_devices->seed; > } > fs_devices->seed = device->fs_devices->seed; > > So presumably fs_devices can never be NULL i.e. the while loop will > always find what it is looking for and so no code to test for NULL is > required. > > > Regards > > David Binderman > > > > _________________________________________________________________ > Do you have a story that started on Hotmail? Tell us now > http://clk.atdmt.com/UKM/go/195013117/direct/01/-- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html