2007-02-10 02:14:45

by Brian Behlendorf

[permalink] [raw]
Subject: e2fsprogs coverity patch <cid-4.diff>

Lawrence Livermore National Labs recently ran the source code
analysis tool Coverity over the e2fsprogs-1.39 source to see
if it would identify any significant bugs. The analysis
turned up 38 mostly minor issues which are enumerated here
with patches. We went through and resolved these issues
but would love to see these mostly minor changes reviewed
and commited upstream.

Thanks,
Brian Behlendorf <[email protected]>, and
Herb Wartens <[email protected]>

-----------------------------------------------------------------------------
Coverity ID: 4: Deadcode

This check is unnecessary since fs_type is guaranteed to be set earlier in the
function.

Index: e2fsprogs+chaos/misc/mke2fs.c
===================================================================
--- e2fsprogs+chaos.orig/misc/mke2fs.c
+++ e2fsprogs+chaos/misc/mke2fs.c
@@ -1310,8 +1310,6 @@ static void PRS(int argc, char *argv[])

if (fs_param.s_feature_incompat &
EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {
- if (!fs_type)
- fs_type = "journal";
reserved_ratio = 0;
fs_param.s_feature_incompat = EXT3_FEATURE_INCOMPAT_JOURNAL_DEV;
fs_param.s_feature_compat = 0;


2007-02-12 19:25:33

by Andreas Dilger

[permalink] [raw]
Subject: Re: e2fsprogs coverity patch <cid-4.diff>

On Feb 09, 2007 18:11 -0800, Brian D. Behlendorf wrote:
> This check is unnecessary since fs_type is guaranteed to be set earlier in the
> function.

I'm not positive this is the right fix. If we are creating a journal device,
we shouldn't be using the normal defaults based on the size of the journal
device because this will often be much smaller than the actual filesystem.

I'd argue that we set fs_type="journal" earlier when fs_type is being first
set, if INCOMPAT_JOURNAL_DEV is set.

> Index: e2fsprogs+chaos/misc/mke2fs.c
> ===================================================================
> --- e2fsprogs+chaos.orig/misc/mke2fs.c
> +++ e2fsprogs+chaos/misc/mke2fs.c
> @@ -1310,8 +1310,6 @@ static void PRS(int argc, char *argv[])
>
> if (fs_param.s_feature_incompat &
> EXT3_FEATURE_INCOMPAT_JOURNAL_DEV) {
> - if (!fs_type)
> - fs_type = "journal";
> reserved_ratio = 0;
> fs_param.s_feature_incompat = EXT3_FEATURE_INCOMPAT_JOURNAL_DEV;
> fs_param.s_feature_compat = 0;

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

2007-03-19 17:02:54

by Theodore Ts'o

[permalink] [raw]
Subject: Re: e2fsprogs coverity patch <cid-04.diff>

On Fri, Feb 09, 2007 at 06:11:35PM -0800, Brian D. Behlendorf wrote:
> This check is unnecessary since fs_type is guaranteed to be set earlier in the
> function.
>
> Coverity ID: 4: Deadcode

The code was actually supposed to be "dead", so in order to restore
the old behavior (from before support for /etc/mke2fs.conf was added)
I needed to move the setting of fs_type of "journal" earlier in the
function.

- Ted