e2fsprogs: a few more warning fixes

PATCH Applied Eric Biggers 2 participants 9 messages Jan 28, 2023 – Jan 30, 2023

Share & tools

Eric Biggers

author

2023-01-28 22:58 UTC #0 lore raw

cover e2fsprogs: a few more warning fixes

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

Signature · 1 line

2.39.1

Eric Biggers

author

2023-01-28 22:58 UTC #1 lore raw

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

From: Eric Biggers <[email protected]>

-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 <[email protected]>

---
 .github/workflows/ci.yml | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

.github/workflows/ci.yml +14 ~~-14~~

index 29482178d..97b15bfbb 100644
@@ -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

Signature · 1 line

2.39.1

Theodore Ts'o

2023-01-30 05:07 UTC #2 lore raw

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

Quoted 11 lines · Eric Biggers

From: Eric Biggers <[email protected]>
-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 <[email protected]>

Applied on the maint branch, thanks.

- Ted

Eric Biggers

author

2023-01-28 22:58 UTC #3 lore raw

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

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

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 <[email protected]>

---
 debugfs/logdump.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

debugfs/logdump.c +2 -1

index 036b50baf..938b48907 100644
@@ -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;
 		}

Signature · 1 line

2.39.1

Theodore Ts'o

2023-01-30 05:08 UTC #4 lore raw

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

Quoted 6 lines · Eric Biggers

From: Eric Biggers <[email protected]>
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 <[email protected]>

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

Eric Biggers

author

2023-01-28 22:58 UTC #5 lore raw

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

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

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

Signed-off-by Eric Biggers <[email protected]>

---
 lib/ext2fs/Android.bp  | 1 -
 lib/ext2fs/ismounted.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

lib/ext2fs/Android.bp +0 -1

index eb4482d79..f5cbeec9f 100644
@@ -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",

lib/ext2fs/ismounted.c +1 -1

index 22bc8352b..a7db1a5c4 100644
@@ -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;

Signature · 1 line

2.39.1

Theodore Ts'o

Applied

2023-01-30 05:09 UTC #6 lore raw

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

Quoted 6 lines · Eric Biggers

From: Eric Biggers <[email protected]>
It is expected that Windows doesn't have getmntent(), so don't warn about it.
Signed-off-by Eric Biggers <[email protected]>

Thanks, applied to the maint branch.

- Ted

Eric Biggers

author

2023-01-28 22:58 UTC #7 lore raw

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

In reply to Eric Biggers

From: Eric Biggers <[email protected]>

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 <[email protected]>

---
 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(-)

lib/uuid/Android.bp +0 -2

index 67968dba3..daf30bb94 100644
@@ -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
         },

lib/uuid/gen_uuid.c +0 -5

index a2225ccee..2f028867a 100644
@@ -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>

lib/uuid/tst_uuid.c +0 -6

index 649bfbc05..c1c290158 100644
@@ -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>

lib/uuid/uuid_time.c +0 -6

index af837a2ca..b519d3c4b 100644
@@ -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>

Signature · 1 line

2.39.1

Theodore Ts'o

Applied

2023-01-30 05:09 UTC #8 lore raw

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

Quoted 14 lines · Eric Biggers

From: Eric Biggers <[email protected]>
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.

Quoted 8 lines

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 <[email protected]>

Thanks, applied to the maint branch.

- Ted