This fixes building dependent packages that use -Werror.
Signed-off-by: Alyssa Ross <[email protected]>
---
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(+)
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;
base-commit: 950a0d69c82b585aba30118f01bf80151deffe8c
--
2.44.0
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 <[email protected]>
> ---
> 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" <[email protected]> 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 <[email protected]>
>> ---
>> 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
This fixes building dependent packages that use -Werror.
Signed-off-by: Alyssa Ross <[email protected]>
---
lib/ext2fs/ext2fs.h | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 6e87829f..ff22f66b 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -585,7 +585,8 @@ typedef struct ext2_struct_inode_scan *ext2_inode_scan;
*/
#define EXT2_I_SIZE(i) ((i)->i_size | ((__u64) (i)->i_size_high << 32))
-static inline __u32 __encode_extra_time(time_t seconds, __u32 nsec)
+static inline __u32 __encode_extra_time(time_t seconds EXT2FS_ATTR((unused)),
+ __u32 nsec)
{
__u32 extra = 0;
@@ -595,7 +596,8 @@ static inline __u32 __encode_extra_time(time_t seconds, __u32 nsec)
#endif
return extra | (nsec << EXT4_EPOCH_BITS);
}
-static inline time_t __decode_extra_sec(time_t seconds, __u32 extra)
+static inline time_t __decode_extra_sec(time_t seconds,
+ __u32 extra EXT2FS_ATTR((unused)))
{
#if (SIZEOF_TIME_T > 4)
if (extra & EXT4_EPOCH_MASK)
@@ -630,7 +632,8 @@ do { \
((struct ext2_inode_large *)(inode))->field ## _extra) : \
(time_t)(inode)->field)
-static inline void __sb_set_tstamp(__u32 *lo, __u8 *hi, time_t seconds)
+static inline void __sb_set_tstamp(__u32 *lo, __u8 *hi EXT2FS_ATTR((unused)),
+ time_t seconds)
{
*lo = seconds & 0xffffffff;
#if (SIZEOF_TIME_T > 4)
@@ -639,7 +642,7 @@ static inline void __sb_set_tstamp(__u32 *lo, __u8 *hi, time_t seconds)
*hi = 0;
#endif
}
-static inline time_t __sb_get_tstamp(__u32 *lo, __u8 *hi)
+static inline time_t __sb_get_tstamp(__u32 *lo, __u8 *hi EXT2FS_ATTR((unused)))
{
#if (SIZEOF_TIME_T == 4)
return *lo;
base-commit: 950a0d69c82b585aba30118f01bf80151deffe8c
--
2.44.0