> 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. Regards, Vlad ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On August 13, 2018 2:23 AM, Theodore Y. Ts'o tytso@xxxxxxx wrote: > On Sun, Aug 12, 2018 at 07:47:15PM +0000, ykp@xxxxxxxxxxxxx wrote: > > > From bbcbf9936c739e53327a9bee4790a167f34fc04a Mon Sep 17 00:00:00 2001 > > From: Vladyslav Tsilytskyi ykp@xxxxxxxxxxxxx > > Date: Sun, 12 Aug 2018 21:34:42 +0200 > > Subject: [PATCH] Fix static analysis warnings. > > Prevented file descriptor leaks in localealias.c > > "!length" code in tdb.c didn't make any sense because variable was > > uninitialized. > > Swapped blocks in dosio.c to prevent "part" variable leak if any > > return from previous block executed. > > None of these changes are actually compiled on Linux. In fact, > dosio.c is never compiled; it's not even mentioned in any of the > Makefiles. The dosio.c file is one that I'm actually quite tempted to > just delete, since it's likely nt_io.c is more likely to be usable. > I'm also extremely tempted to just delete all of the intl directory. > That directory is never compiled for Linux, since the libintl library > is provided by glibc. And it's badly out of date. It's from > gettext-0.14.1, and the latest version of gettext is 0.19.1. The > problem is integrating the intl sources into the e2fsprogs build > system. It's not hard work per se, but it's finicky, and it's only > useful if you want to build e2fsprogs with I18N support on say, > Solaris, AIX, NetBSD, etc. And I just don't care that much. The more > we diverge from the intl sources from the GNU gettext page, the harder > it's going to sync up with newer versions. > Fixing these sorts of problems really isn't worth it. That's because > I really don't like to apply super-complex patches without very > careful desk checking, especially if I can't test the changes easily. > And these are just resource leaks on error cases, and none of these > are likely to be serious leaks (e.g., they are one-off leaks, and not > something that would be continually leaking and causing long-running > programs to eventually run out of memory). The argument of silencing > warnings to make it easier to spot "real" bugs is also not very > compelling for cppcheck, because the level of checking it does is very > low quality. We can do much better using GCC -Wall, Clang warnings, > and Coverity. So.... why? > The tdb patch does fix a real bug, although it's completely irrelevant > for Linux (since we have strdup, some we don't need the replacement). > Fortunately it's a simple enough patch that it's obviously correct > from inspection. This is why I told you privately this was the only > fix that I considered worth it. > Cheers, > > - Ted
