| Message ID | 20240527091542.4121237-2-hi@alyssa.is |
|---|---|
| State | New |
| Headers | show |
| Series | libext2fs: fix unused parameter warnings/errors | expand |
On Mon, May 27, 2024 at 11:15:43AM +0200, Alyssa Ross wrote: > This fixes building dependent packages that use -Werror. > > Signed-off-by: Alyssa Ross <hi@alyssa.is> > --- > I'm assuming here that it is actually intentional that these variables > are unused! I don't understand the code enough to know for sure — > I'm just trying to fix some build regressions after updating e2fsprogs. :) Well, note that you only get the warning at all if you use -Wunused-parameter, "-Wunused -Wextra", or "-Wall -Wextra". The unused-parameter warning is not enabled unless you **really** go out of your way to enable it, because it has so many false positives. The gcc maintainers do not enable this insanity even with -Wall, so someone really went out of their way to make your life miserable. :-) I generally think it's a really bad idea to turn on warnings as if they are overdone Christmas tree lights. However, to accomodate this, the normal way to suppress this is via __attribute__(unused). To do this in a portable way to avoid breaking compilers which don't understand said attribute: /* this is usually predfined in some header file like compiler.h */ #ifdef __GNUC__ #define EXT2FS_ATTR(x) __attribute__(x) #else #define EXT2FS_ATTR(x) #endif ... _INLINE_ errcode_t ext2fs_resize_mem(unsigned long EXT2FS_ATTR((unused)) old_size, unsigned long size, void *ptr) ... You can also play this game if you really have a huge number of stupid gcc warnings that you want to suppress: /* this is usually predfined in some header file like compiler.h */ #ifndef __GNUC_PREREQ #if defined(__GNUC__) && defined(__GNUC_MINOR__) #define __GNUC_PREREQ(maj, min) \ ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min)) #else #define __GNUC_PREREQ(maj, min) 0 #endif #endif #if __GNUC_PREREQ (4, 6) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wunused-function" #pragma GCC diagnostic ignored "-Wunused-parameter" #endif ... #if __GNUC_PREREQ (4, 6) #pragma GCC diagnostic pop #endif I do this when I'm defining a lot of inline functions in a header file, and some stupid person thinks that -Wunused -Wextra is actually a good idea (hint: it's not), and I just want to shut up the completely unnecessary warnings. Cheers, - Ted
"Theodore Ts'o" <tytso@mit.edu> writes: > On Mon, May 27, 2024 at 11:15:43AM +0200, Alyssa Ross wrote: >> This fixes building dependent packages that use -Werror. >> >> Signed-off-by: Alyssa Ross <hi@alyssa.is> >> --- >> I'm assuming here that it is actually intentional that these variables >> are unused! I don't understand the code enough to know for sure — >> I'm just trying to fix some build regressions after updating e2fsprogs. :) > > Well, note that you only get the warning at all if you use > -Wunused-parameter, "-Wunused -Wextra", or "-Wall -Wextra". The > unused-parameter warning is not enabled unless you **really** go out > of your way to enable it, because it has so many false positives. The > gcc maintainers do not enable this insanity even with -Wall, so > someone really went out of their way to make your life miserable. :-) Yeah… https://github.com/storaged-project/libblockdev/blob/7cd19d14e61c8964187eac99cf276e6c999dc93e/src/plugins/fs/Makefile.am#L5 (In this case it /did/ end up with me having to look at the code and notice the SIZEOF_SIZE_T bug I sent another patch for, so something useful did actually come out of it this time…) > I generally think it's a really bad idea to turn on warnings as if > they are overdone Christmas tree lights. However, to accomodate this, > the normal way to suppress this is via __attribute__(unused). To do > this in a portable way to avoid breaking compilers which don't > understand said attribute: > > /* this is usually predfined in some header file like compiler.h */ > #ifdef __GNUC__ > #define EXT2FS_ATTR(x) __attribute__(x) > #else > #define EXT2FS_ATTR(x) > #endif > > ... > _INLINE_ errcode_t ext2fs_resize_mem(unsigned long EXT2FS_ATTR((unused)) old_size, > unsigned long size, void *ptr) > ... Okay, I'll send a v2 using this approach. > You can also play this game if you really have a huge number of stupid > gcc warnings that you want to suppress: > > /* this is usually predfined in some header file like compiler.h */ > #ifndef __GNUC_PREREQ > #if defined(__GNUC__) && defined(__GNUC_MINOR__) > #define __GNUC_PREREQ(maj, min) \ > ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min)) > #else > #define __GNUC_PREREQ(maj, min) 0 > #endif > #endif > > #if __GNUC_PREREQ (4, 6) > #pragma GCC diagnostic push > #pragma GCC diagnostic ignored "-Wunused-function" > #pragma GCC diagnostic ignored "-Wunused-parameter" > #endif > > ... > > #if __GNUC_PREREQ (4, 6) > #pragma GCC diagnostic pop > #endif > > I do this when I'm defining a lot of inline functions in a header > file, and some stupid person thinks that -Wunused -Wextra is actually > a good idea (hint: it's not), and I just want to shut up the > completely unnecessary warnings. > > Cheers, > > - Ted
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 6e87829f..a1ce192b 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -592,6 +592,8 @@ static inline __u32 __encode_extra_time(time_t seconds, __u32 nsec) #if (SIZEOF_TIME_T > 4) extra = ((seconds - (__s32)(seconds & 0xffffffff)) >> 32) & EXT4_EPOCH_MASK; +#else + (void)seconds; #endif return extra | (nsec << EXT4_EPOCH_BITS); } @@ -600,6 +602,8 @@ static inline time_t __decode_extra_sec(time_t seconds, __u32 extra) #if (SIZEOF_TIME_T > 4) if (extra & EXT4_EPOCH_MASK) seconds += ((time_t)(extra & EXT4_EPOCH_MASK) << 32); +#else + (void)extra; #endif return seconds; } @@ -642,6 +646,7 @@ static inline void __sb_set_tstamp(__u32 *lo, __u8 *hi, time_t seconds) static inline time_t __sb_get_tstamp(__u32 *lo, __u8 *hi) { #if (SIZEOF_TIME_T == 4) + (void)hi; return *lo; #else return ((time_t)(*hi) << 32) | *lo;
This fixes building dependent packages that use -Werror. Signed-off-by: Alyssa Ross <hi@alyssa.is> --- I'm assuming here that it is actually intentional that these variables are unused! I don't understand the code enough to know for sure — I'm just trying to fix some build regressions after updating e2fsprogs. :) lib/ext2fs/ext2fs.h | 5 +++++ 1 file changed, 5 insertions(+) base-commit: 950a0d69c82b585aba30118f01bf80151deffe8c