[PATCH 0/4] e2fsprogs: a few more warning fixes

[PATCH 0/4] e2fsprogs: a few more warning fixes

[Eric Biggers]

Fix the GitHub Actions workflow to really use -Werror when building the
libraries, fix a recently introduced warning in debugfs, and fix two
warnings in the Windows build.

Eric Biggers (4):
  ci.yml: ensure -Werror really gets used in all cases
  debugfs: fix a -Wformat warning in dump_journal()
  lib/ext2fs: don't warn about lack of getmntent on Windows
  lib/uuid: remove unneeded Windows UUID workaround

 .github/workflows/ci.yml | 28 ++++++++++++++--------------
 debugfs/logdump.c        |  3 ++-
 lib/ext2fs/Android.bp    |  1 -
 lib/ext2fs/ismounted.c   |  2 +-
 lib/uuid/Android.bp      |  2 --
 lib/uuid/gen_uuid.c      |  5 -----
 lib/uuid/tst_uuid.c      |  6 ------
 lib/uuid/uuid_time.c     |  6 ------
 8 files changed, 17 insertions(+), 36 deletions(-)


base-commit: 0352d353adbe6c5d6f1937e12c66e599b8657d72
-- 
2.39.1

[PATCH 1/4] ci.yml: ensure -Werror really gets used in all cases

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

-Werror wasn't actually being used when building the libraries, as the
libraries use CFLAGS_STLIB instead of CFLAGS.

Use CFLAGS_WARN, which gets included in both.

Note: -Werror can't just be passed to 'configure' like the other flags
are, as it interferes with some of the configure checks.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 .github/workflows/ci.yml | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 29482178d..97b15bfbb 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -10,7 +10,7 @@ jobs:
     steps:
     - uses: actions/checkout@v2
     - run: ./configure CC=gcc CFLAGS="$DEF_CFLAGS"
-    - run: make -j8 check V=1 CFLAGS="$DEF_CFLAGS -Werror"
+    - run: make -j8 check V=1 CFLAGS_WARN="-Werror"
     - run: make -j8 install V=1 DESTDIR=$PWD/installdir
     - run: make -j8 uninstall V=1 DESTDIR=$PWD/installdir
 
@@ -24,7 +24,7 @@ jobs:
         sudo apt-get update
         sudo apt-get install -y clang
     - run: ./configure CC=clang CFLAGS="$DEF_CFLAGS"
-    - run: make -j8 check V=1 CFLAGS="$DEF_CFLAGS -Werror"
+    - run: make -j8 check V=1 CFLAGS_WARN="-Werror"
     - run: make -j8 install V=1 DESTDIR=$PWD/installdir
     - run: make -j8 uninstall V=1 DESTDIR=$PWD/installdir
 
@@ -38,7 +38,7 @@ jobs:
         sudo apt-get update
         sudo apt-get install -y gcc-multilib
     - run: ./configure CC=gcc CFLAGS="$DEF_CFLAGS -m32" LDFLAGS="-m32"
-    - run: make -j8 check V=1 CFLAGS="$DEF_CFLAGS -m32 -Werror"
+    - run: make -j8 check V=1 CFLAGS_WARN="-Werror"
     - run: make -j8 install V=1 DESTDIR=$PWD/installdir
     - run: make -j8 uninstall V=1 DESTDIR=$PWD/installdir
 
@@ -53,7 +53,7 @@ jobs:
         sudo apt-get install -y clang
     - run: echo "ASAN_CFLAGS=$DEF_CFLAGS -fsanitize=address -fno-sanitize-recover=address" >> $GITHUB_ENV
     - run: ./configure CC=clang CFLAGS="$ASAN_CFLAGS" LDFLAGS="$ASAN_CFLAGS"
-    - run: make -j8 check V=1 CFLAGS="$ASAN_CFLAGS -Werror"
+    - run: make -j8 check V=1 CFLAGS_WARN="-Werror"
 
   ubsan-build-and-test:
     name: Build and test with UBSAN enabled
@@ -66,7 +66,7 @@ jobs:
         sudo apt-get install -y clang
     - run: echo "UBSAN_CFLAGS=$DEF_CFLAGS -fsanitize=undefined -fno-sanitize-recover=undefined" >> $GITHUB_ENV
     - run: ./configure CC=clang CFLAGS="$UBSAN_CFLAGS" LDFLAGS="$UBSAN_CFLAGS"
-    - run: make -j8 check V=1 CFLAGS="$UBSAN_CFLAGS -Werror"
+    - run: make -j8 check V=1 CFLAGS_WARN="-Werror"
 
   macos-build-and-test:
     name: Build and test on macOS
@@ -76,7 +76,7 @@ jobs:
     - run: ./configure CFLAGS="$DEF_CFLAGS"
       # -Wno-error=deprecated-declarations is needed to suppress known warnings
       # due to e2fsprogs' use of sbrk(0) and daemon().
-    - run: make -j8 check V=1 CFLAGS="$DEF_CFLAGS -Werror -Wno-error=deprecated-declarations"
+    - run: make -j8 check V=1 CFLAGS_WARN="-Werror -Wno-error=deprecated-declarations"
     - run: make -j8 install DESTDIR=$PWD/installdir
     - run: make -j8 uninstall DESTDIR=$PWD/installdir
 
@@ -104,13 +104,13 @@ jobs:
     # dependencies: all libraries except libss.  The build system doesn't want
     # to build just those parts, though, so do it one step at a time...
     - run: ./configure CFLAGS="$DEF_CFLAGS"
-    - run: make -j8 subs V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C lib/et/ all V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C lib/uuid/ all V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C lib/blkid/ all V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C lib/ext2fs/ all V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C lib/support/ all V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C lib/e2p/ all V=1 CFLAGS="$DEF_CFLAGS -Werror"
-    - run: make -j8 -C misc/ mke2fs V=1 CFLAGS="$DEF_CFLAGS -Werror"
+    - run: make -j8 subs V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C lib/et/ all V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C lib/uuid/ all V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C lib/blkid/ all V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C lib/ext2fs/ all V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C lib/support/ all V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C lib/e2p/ all V=1 CFLAGS_WARN="-Werror"
+    - run: make -j8 -C misc/ mke2fs V=1 CFLAGS_WARN="-Werror"
     - run: touch image.ext4
     - run: misc/mke2fs.exe -T ext4 image.ext4 128M

base-commit: 0352d353adbe6c5d6f1937e12c66e599b8657d72
-- 
2.39.1

[PATCH 2/4] debugfs: fix a -Wformat warning in dump_journal()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

This really should just use PRIi64, but e2fsprogs doesn't use the
inttypes.h format specifiers elsewhere, so just be consistent for now.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 debugfs/logdump.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/debugfs/logdump.c b/debugfs/logdump.c
index 036b50baf..938b48907 100644
--- a/debugfs/logdump.c
+++ b/debugfs/logdump.c
@@ -479,7 +479,8 @@ static void dump_journal(char *cmdname, FILE *out_file,
 
 		if ((blocknr == first_transaction_blocknr) &&
 		    (cur_counts != 0) && dump_old && (dump_counts != -1)) {
-			fprintf(out_file, "Dump all %lld journal records.\n", cur_counts);
+			fprintf(out_file, "Dump all %lld journal records.\n",
+				(long long)cur_counts);
 			break;
 		}
 
-- 
2.39.1

[PATCH 3/4] lib/ext2fs: don't warn about lack of getmntent on Windows

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

It is expected that Windows doesn't have getmntent(), so don't warn
about it.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 lib/ext2fs/Android.bp  | 1 -
 lib/ext2fs/ismounted.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/ext2fs/Android.bp b/lib/ext2fs/Android.bp
index eb4482d79..f5cbeec9f 100644
--- a/lib/ext2fs/Android.bp
+++ b/lib/ext2fs/Android.bp
@@ -121,7 +121,6 @@ cc_library {
             enabled: true,
             include_dirs: ["external/e2fsprogs/include/mingw"],
             cflags: [
-                "-Wno-error=cpp",
                 "-Wno-format",
                 "-Wno-unused-variable",
                 "-Wno-error=typedef-redefinition",
diff --git a/lib/ext2fs/ismounted.c b/lib/ext2fs/ismounted.c
index 22bc8352b..a7db1a5c4 100644
--- a/lib/ext2fs/ismounted.c
+++ b/lib/ext2fs/ismounted.c
@@ -414,7 +414,7 @@ errcode_t ext2fs_check_mount_point(const char *device, int *mount_flags,
 #ifdef HAVE_GETMNTINFO
 		retval = check_getmntinfo(device, mount_flags, mtpt, mtlen);
 #else
-#ifdef __GNUC__
+#if defined(__GNUC__) && !defined(_WIN32)
  #warning "Can't use getmntent or getmntinfo to check for mounted filesystems!"
 #endif
 		*mount_flags = 0;
-- 
2.39.1

[PATCH 4/4] lib/uuid: remove unneeded Windows UUID workaround

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Some .c files in lib/uuid/ contain the following:

	#ifdef _WIN32
	#define _WIN32_WINNT 0x0500
	#include <windows.h>
	#define UUID MYUUID
	#endif

This seems to have been intended to allow the use of a local "UUID" type
without colliding with "UUID" in the Windows API.  However, this is
unnecessary because there's no local "UUID" type -- there's only uuid_t.

None of these .c files need the include of windows.h, either.

Finally, the unconditional definition of _WIN32_WINNT causes a compiler
warning when the user defines _WIN32_WINNT themself.

Since this code is unnecessary and is causing problems, just remove it.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 lib/uuid/Android.bp  | 2 --
 lib/uuid/gen_uuid.c  | 5 -----
 lib/uuid/tst_uuid.c  | 6 ------
 lib/uuid/uuid_time.c | 6 ------
 4 files changed, 19 deletions(-)

diff --git a/lib/uuid/Android.bp b/lib/uuid/Android.bp
index 67968dba3..daf30bb94 100644
--- a/lib/uuid/Android.bp
+++ b/lib/uuid/Android.bp
@@ -45,8 +45,6 @@ cc_library {
     ],
     target: {
         windows: {
-            // Cannot suppress the _WIN32_WINNT redefined warning.
-            cflags: ["-Wno-error"],
             include_dirs: [ "external/e2fsprogs/include/mingw" ],
             enabled: true
         },
diff --git a/lib/uuid/gen_uuid.c b/lib/uuid/gen_uuid.c
index a2225ccee..2f028867a 100644
--- a/lib/uuid/gen_uuid.c
+++ b/lib/uuid/gen_uuid.c
@@ -41,11 +41,6 @@
 
 #include "config.h"
 
-#ifdef _WIN32
-#define _WIN32_WINNT 0x0500
-#include <windows.h>
-#define UUID MYUUID
-#endif
 #include <stdio.h>
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
diff --git a/lib/uuid/tst_uuid.c b/lib/uuid/tst_uuid.c
index 649bfbc05..c1c290158 100644
--- a/lib/uuid/tst_uuid.c
+++ b/lib/uuid/tst_uuid.c
@@ -34,12 +34,6 @@
 
 #include "config.h"
 
-#ifdef _WIN32
-#define _WIN32_WINNT 0x0500
-#include <windows.h>
-#define UUID MYUUID
-#endif
-
 #include <stdio.h>
 #include <stdlib.h>
 
diff --git a/lib/uuid/uuid_time.c b/lib/uuid/uuid_time.c
index af837a2ca..b519d3c4b 100644
--- a/lib/uuid/uuid_time.c
+++ b/lib/uuid/uuid_time.c
@@ -36,12 +36,6 @@
 
 #include "config.h"
 
-#ifdef _WIN32
-#define _WIN32_WINNT 0x0500
-#include <windows.h>
-#define UUID MYUUID
-#endif
-
 #include <stdio.h>
 #ifdef HAVE_UNISTD_H
 #include <unistd.h>
-- 
2.39.1

Re: [PATCH 1/4] ci.yml: ensure -Werror really gets used in all cases

[Theodore Ts'o]

On Sat, Jan 28, 2023 at 02:46:48PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> -Werror wasn't actually being used when building the libraries, as the
> libraries use CFLAGS_STLIB instead of CFLAGS.
> 
> Use CFLAGS_WARN, which gets included in both.
> 
> Note: -Werror can't just be passed to 'configure' like the other flags
> are, as it interferes with some of the configure checks.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Applied on the maint branch, thanks.

					- Ted

Re: [PATCH 2/4] debugfs: fix a -Wformat warning in dump_journal()

[Theodore Ts'o]

On Sat, Jan 28, 2023 at 02:46:49PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> This really should just use PRIi64, but e2fsprogs doesn't use the
> inttypes.h format specifiers elsewhere, so just be consistent for now.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, I have this in my tree already (but I just hadn't pushed it
out yet; I wanted to get some other fixes done on the maint branch
before I merged maint with the master/next branches.)

       	 	      	       		   - Ted

Re: [PATCH 3/4] lib/ext2fs: don't warn about lack of getmntent on Windows

[Theodore Ts'o]

On Sat, Jan 28, 2023 at 02:46:50PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> It is expected that Windows doesn't have getmntent(), so don't warn
> about it.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied to the maint branch.

					- Ted

Re: [PATCH 4/4] lib/uuid: remove unneeded Windows UUID workaround

[Theodore Ts'o]

On Sat, Jan 28, 2023 at 02:46:51PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Some .c files in lib/uuid/ contain the following:
> 
> 	#ifdef _WIN32
> 	#define _WIN32_WINNT 0x0500
> 	#include <windows.h>
> 	#define UUID MYUUID
> 	#endif
> 
> This seems to have been intended to allow the use of a local "UUID" type
> without colliding with "UUID" in the Windows API.  However, this is
> unnecessary because there's no local "UUID" type -- there's only uuid_t.
> 
> None of these .c files need the include of windows.h, either.
> 
> Finally, the unconditional definition of _WIN32_WINNT causes a compiler
> warning when the user defines _WIN32_WINNT themself.
> 
> Since this code is unnecessary and is causing problems, just remove it.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied to the maint branch.

					- Ted