Message-ID: <H-DxQTaMH81SEHpg36eDsEkLs4yLyH9mgIxW8c4gtsaGPC5agwtFJcNGVHTZbxGpkYoRGvyLGu3xpbYlB6hpX6nzqX4aiVelavVdFEg10XQ=@protonmail.ch>
Date:   Wed, 15 Aug 2018 13:56:42 +0000
From:   ykp@...tonmail.ch
To:     "Theodore Y. Ts'o" <tytso@....edu>
Cc:     "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        Jaco Kroon <jaco@....co.za>
Subject: Re: [PATCH] ext2/e2fsprogs: fix cppcheck warnings

Ted,

gcc-wall generate a lot of noice, for example 45 such warnings in /debugfs/debugfs.c:

> ../../debugfs/debugfs.c:221:49: warning: declaration of ‘sci_idx’ shadows a global declaration [-Wshadow]
> void do_open_filesys(int argc, char **argv, int sci_idx EXT2FS_ATTR((unused)),
> ../../debugfs/debugfs.h:28:12: note: shadowed declaration is here
> extern int sci_idx;
> ^~~~~~~

Of course I can patch all these places like this:

> diff --git a/debugfs/debugfs.c b/debugfs/debugfs.c
> index e03519c4..6d0e9cdb 100644
> --- a/debugfs/debugfs.c
> +++ b/debugfs/debugfs.c
> @@ -218,7 +218,7 @@ errout:
>  		com_err(device, retval, "while trying to close filesystem");
>  }
>
> -void do_open_filesys(int argc, char **argv, int sci_idx EXT2FS_ATTR((unused)),
> +void do_open_filesys(int argc, char **argv, int _sci_idx EXT2FS_ATTR((unused)),
>  		     void *infop EXT2FS_ATTR((unused)))
>  {
>  	int	c, err;

But is it worth to?

I've tried clang as you suggested.
I've replaced "CC = gcc" by "CC = clang" or even "CC = clang -Weverything -pedantic",
but output remains the same as if I just normally compile the project with make and gcc.
What is wrong?

Regards,
Vlad

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On August 13, 2018 4:50 PM, ykp@...tonmail.ch wrote:

> Thank you for pointing places where some job can be done.
> I'll definitely check it and do my best to clean it up.
>
> > Is there something specific you are interested in working on?
>
> My original goal was to find out how fsck works, check if there
> is a standard API for all set of fsck.* utilities and as a result
> write a new one for fuse.cryfs filesystem.
> Regards,
> Vlad
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On August 13, 2018 4:37 PM, Theodore Y. Ts'o tytso@....edu wrote:
>
> > On Mon, Aug 13, 2018 at 08:50:52AM +0000, ykp@...tonmail.ch wrote:
> >
> > > > So.... why?
> > >
> > > There is no great reason behind. I believe that evidently buggy code
> > > needs to be fixed (or removed).
> > > Yeah, cppcheck is not the best tool. In this case it was a way for me
> > > to get along with both: e2fsprogs (I need a starting point to explore
> > > the code) and cppcheck. I'm not going to run this static analysis
> > > tool on a regular basis, treat it as a learning step.
> >
> > If you want to do code cleanup, it's better to either look for Clang
> > warnings or gcc-wall warnings. The first can be done via "CC=clang
> > configure". The second can be done via running "make gcc-wall" in a
> > particular build directory. After you fix gcc-wall issues, you can
> > run "make gcc-wall-new" to only run gcc -Wall on the modified files.
> > You can run the test_script in the tests directory with the --valgrind
> > or --valgrind-leakcheck.
> > In some cases we've deliberately neelded not fixed a warning when it's
> > not worth it. Long-term maintainability and code readability is
> > important.
> > One file where a lot of cleanup can be needed --- not just blindly
> > cleaning up gcc -Wall or clang warnings, but rather restructing and
> > general code cleanup to make the code cleaner and consistent with
> > general e2fsprogs code quality and style --- is misc/e4defrag.c.
> > There is some interest by Jaco to add new featuers on e4defrag, so if
> > that is something you are interested in doing somme cleanup work on,
> > we'll need to do some air traffic control to avoid change conflicts.
> > Is there something specific you are interested in working on?
> > Finally, one potential issue. Since you are working under an
> > encrypted channel and you aren't specifying your name, I assume you
> > are concerned about preserving your anonymity. One of the problems is
> > that if you are making code contributions, I need to know that who you
> > are. It doesn't have to be public --- you can let me know in private
> > --- but I do need to know your identity. There is precedence for this
> > --- "George Spellvin" is an occasional contributor to the Linux
> > kernel, but Linus Torvalds know who he is, and that's been considered
> > sufficient. Please see the description of the Developers
> > Certification of Origin (e.g., the "Signed-off-by" header) for the
> > background about what it is that we require code contributors to agree
> > when they contribute code.
> > Cheers,
> >
> > -   Ted

Powered by blists - more mailing lists