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@xxxxxxxxxxxxx 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@xxxxxxx wrote:
>
> > On Mon, Aug 13, 2018 at 08:50:52AM +0000, ykp@xxxxxxxxxxxxx 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
