-Werror, more warnings, and warning fixes

Author: cebtenzzre

SConstruct

@@ -127,6 +127,17 @@ def check_sysmacro_h(context):
     return rc
 
 
+def check_stdnoreturn_h(context):
+    rc = 1
+    if rc and tests.CheckHeader(context, 'stdnoreturn.h'):
+        rc = 0
+
+    conf.env['HAVE_STDNORETURN_H'] = rc
+    context.did_show_result = True
+    context.Result(rc)
+    return rc
+
+
 def check_libelf(context):
     rc = 1
 
@@ -618,7 +629,8 @@ conf = Configure(env, custom_tests={
     'check_mm_crc32_u64': check_mm_crc32_u64,
     'check_cpu_extensions': check_cpu_extensions,
     'check_builtin_cpu_supports': check_builtin_cpu_supports,
-    'check_sysmacro_h': check_sysmacro_h
+    'check_sysmacro_h': check_sysmacro_h,
+    'check_stdnoreturn_h': check_stdnoreturn_h
 })
 
 #######################################################################
@@ -706,20 +718,33 @@ conf.check_mm_crc32_u64()
 if any(cc in os.path.basename(conf.env['CC']) for cc in ('clang', 'include-what-you-use')):
     conf.env.Append(CCFLAGS=['-fcolor-diagnostics'])  # Colored warnings
     conf.env.Append(CCFLAGS=['-Qunused-arguments'])   # Hide wrong messages
-    conf.env.Append(CCFLAGS=['-Wno-bad-function-cast'])
+    conf.env.Append(CCFLAGS=[
+        '-Wmost',
+        '-Wunreachable-code-aggressive',
+        '-Wno-bad-function-cast',
+    ])
 else:
     gcc_version = conf.check_gcc_version()
+    if gcc_version:
+        conf.env.Append(CCFLAGS=[
+            '-Wduplicated-cond',
+            '-Wduplicated-branches',
+            '-Wlogical-op',
+        ])
     if gcc_version >= 8:
         conf.env.Append(CCFLAGS=['-Wno-cast-function-type'])
 
 # Optional flags:
 conf.env.Append(CCFLAGS=[
-    '-Wall', '-W', '-Wextra',
+    '-Wall', '-Wextra',
     '-Winit-self',
-    '-Wstrict-aliasing',
     '-Wmissing-include-dirs',
     '-Wuninitialized',
     '-Wstrict-prototypes',
+    '-Wnull-dereference',
+    '-Wformat-security',
+    '-Wformat-y2k',
+    '-Wmissing-noreturn',
     '-Wno-implicit-fallthrough',
 ])
 
@@ -745,6 +770,7 @@ conf.check_btrfs_h()
 conf.check_linux_fs_h()
 conf.check_uname()
 conf.check_sysmacro_h()
+conf.check_stdnoreturn_h()
 conf.check_cpu_extensions()
 
 if conf.env['HAVE_LIBELF']:
@@ -763,7 +789,7 @@ if conf.env['HAVE_SSE2']:
 	conf.env.Append(CCFLAGS=['-msse2'])
 
 # NB: After checks so they don't fail
-conf.env.Append(CCFLAGS=['-Werror=undef'])
+conf.env.Append(CCFLAGS=['-Werror'])
 
 
 if ARGUMENTS.get('GDB') == '1':

lib/SConscript

@@ -42,6 +42,7 @@ def build_config_template(target, source, env):
             HAVE_BUILTIN_CPU_SUPPORTS=env['HAVE_BUILTIN_CPU_SUPPORTS'],
             HAVE_UNAME=env['HAVE_UNAME'],
             HAVE_SYSMACROS_H=env['HAVE_SYSMACROS_H'],
+            HAVE_STDNORETURN_H=env['HAVE_STDNORETURN_H'],
             VERSION_MAJOR=VERSION_MAJOR,
             VERSION_MINOR=VERSION_MINOR,
             VERSION_PATCH=VERSION_PATCH,

lib/checksum.c

@@ -438,7 +438,7 @@ static void rm_digest_glib_steal(GChecksum *state, guint8 *result, gsize *len) {
         .free = (RmDigestFreeFunc)g_checksum_free,       \
         .update = (RmDigestUpdateFunc)g_checksum_update, \
         .copy = (RmDigestCopyFunc)g_checksum_copy,       \
-        .steal = (RmDigestStealFunc)rm_digest_##NAME##_steal};
+        .steal = (RmDigestStealFunc)rm_digest_##NAME##_steal}
 
 /* md5 */
 static GChecksum *rm_digest_md5_new(void) {
@@ -587,7 +587,7 @@ RM_DIGEST_DEFINE_SHA3(512)
         .free = (RmDigestFreeFunc)rm_digest_##ALGO##_free,                      \
         .update = (RmDigestUpdateFunc)ALGO##_update,                            \
         .copy = (RmDigestCopyFunc)rm_digest_##ALGO##_copy,                      \
-        .steal = (RmDigestStealFunc)rm_digest_##ALGO##_steal};
+        .steal = (RmDigestStealFunc)rm_digest_##ALGO##_steal}
 
 CREATE_BLAKE2_INTERFACE(blake2b, BLAKE2B);
 CREATE_BLAKE2_INTERFACE(blake2bp, BLAKE2B);

lib/cmdline.c

@@ -28,6 +28,7 @@
 #include <string.h>
 #include <search.h>
 
+#include "config.h"
 #include "formats.h"
 #include "md-scheduler.h"
 #include "preprocess.h"
@@ -51,7 +52,7 @@ static const RmDigestType RM_PARANOIA_LEVELS[] = {RM_DIGEST_METRO,
 static const int RM_PARANOIA_NORMAL = 3;  /*  must be index of RM_DEFAULT_DIGEST */
 static const int RM_PARANOIA_MAX = 5;
 
-static void rm_cmd_show_version(void) {
+NORETURN static void rm_cmd_show_version(void) {
     fprintf(stderr, "version %s compiled: %s at [%s] \"%s\" (rev %s)\n", RM_VERSION,
             __DATE__, __TIME__, RM_VERSION_NAME, RM_VERSION_GIT_REVISION);
 
@@ -86,20 +87,20 @@ static void rm_cmd_show_version(void) {
     exit(0);
 }
 
-static void rm_cmd_show_manpage(void) {
-    static const char *commands[] = {"man %s docs/_build/man/rmlint.1 2> /dev/null",
-                                     "man %s rmlint", NULL};
+#if RM_MANPAGE_USE_PAGER
+    #define MAN_OPTS ""
+#else
+    #define MAN_OPTS "-P cat"
+#endif
+
+NORETURN static void rm_cmd_show_manpage(void) {
+    static const char *commands[] = {"man " MAN_OPTS " docs/_build/man/rmlint.1.gz 2> /dev/null",
+                                     "man " MAN_OPTS " rmlint", NULL};
 
     bool found_manpage = false;
 
     for(int i = 0; commands[i] && !found_manpage; ++i) {
-        char cmd_buf[512] = {0};
-        if(snprintf(cmd_buf, sizeof(cmd_buf), commands[i],
-                    (RM_MANPAGE_USE_PAGER) ? "" : "-P cat") == -1) {
-            continue;
-        }
-
-        if(system(cmd_buf) == 0) {
+        if(system(commands[i]) == 0) {
             found_manpage = true;
         }
     }

lib/config.h.in

@@ -21,6 +21,7 @@
 #define HAVE_LINUX_FS_H    ({HAVE_LINUX_FS_H})
 #define HAVE_UNAME         ({HAVE_UNAME})
 #define HAVE_SYSMACROS_H   ({HAVE_SYSMACROS_H})
+#define HAVE_STDNORETURN_H ({HAVE_STDNORETURN_H})
 #define HAVE_MM_CRC32_U64  ({HAVE_MM_CRC32_U64})
 #define HAVE_BUILTIN_CPU_SUPPORTS ({HAVE_BUILTIN_CPU_SUPPORTS})
 
@@ -100,7 +101,7 @@ typedef guint64 RmOff;
     #endif
     #ifdef __GNUC__
         #define INLINE inline __attribute__((__always_inline__))
-    #elif defined(__CLANG__)
+    #elif defined(__clang__)
         #if __has_attribute(__always_inline__)
             #define INLINE inline __attribute__((__always_inline__))
         #endif
@@ -115,13 +116,27 @@ typedef guint64 RmOff;
 
 #ifdef __GNUC__
     #define WARN_UNUSED_RESULT __attribute__((warn_unused_result))
-#elif defined(__CLANG__)
+#elif defined(__clang__)
     #define WARN_UNUSED_RESULT __attribute__((warn_unused_result))
 #else
     /* give up */
     #define WARN_UNUSED_RESULT
 #endif
 
+#if HAVE_STDNORETURN_H
+    #include <stdnoreturn.h>
+#endif
+
+#ifdef noreturn
+    #define NORETURN noreturn
+#elif defined(__GNUC__) || defined(__clang__)
+    #define NORETURN __attribute__((__noreturn__))
+#elif defined(_MSC_VER)
+    #define NORETURN __declspec(noreturn)
+#else
+    #define noreturn
+#endif
+
 #define _RM_OFFSET_DEBUG 0
 
 #endif

lib/formats/sh.c.in

@@ -45,8 +45,8 @@ typedef struct RmFmtHandlerShScript {
     RmOff line_count;
 } RmFmtHandlerShScript;
 
-static const char *SH_SCRIPT_TEMPLATE_HEAD = "<<SH_SOURCE>>";
-static const char *SH_SCRIPT_TEMPLATE_FOOT =
+static const char SH_SCRIPT_TEMPLATE_HEAD[] = "<<SH_SOURCE>>";
+static const char SH_SCRIPT_TEMPLATE_FOOT[] =
     "\n"
     "\n"
     "\n"

lib/formats/stats.c

@@ -96,7 +96,7 @@ static void rm_fmt_prog(RmSession *session,
     fprintf(out, _("%s%15ld%s Other lint items\n"), MAYBE_RED(out, session),
             (long)session->other_lint_cnt, MAYBE_RESET(out, session));
 
-    gfloat elapsed = g_timer_elapsed(session->timer_since_proc_start, NULL);
+    gdouble elapsed = g_timer_elapsed(session->timer_since_proc_start, NULL);
 
     char *elapsed_time = rm_format_elapsed_time(elapsed, 5);
     fprintf(out, _("%s%15s%s of time spent scanning\n"), MAYBE_RED(out, session),

lib/formats/summary.c

@@ -107,7 +107,7 @@ static void rm_fmt_prog(RmSession *session,
                       _("This run was a replay from a previous run. No I/O done!\n"));
     }
 
-    gfloat elapsed = g_timer_elapsed(session->timer_since_proc_start, NULL);
+    gdouble elapsed = g_timer_elapsed(session->timer_since_proc_start, NULL);
     char *elapsed_time = rm_format_elapsed_time(elapsed, 3);
     ARROW fprintf(out, _("Scanning took in total %s%s%s.\n"), MAYBE_RED(out, session),
                   elapsed_time, MAYBE_RESET(out, session));

lib/pathtricia.h

@@ -53,7 +53,7 @@ typedef struct _RmNode {
     ino_t inode;
 
     /* data was set explicitly */
-    char has_value : 1;
+    bool has_value : 1;
 
     /* User specific data */
     gpointer data;

lib/reflink.c

@@ -167,6 +167,8 @@ RmLinkType rm_reflink_type_from_fd(int fd1, int fd2) {
 
     return RM_LINK_ERROR;
 #else
+    (void)fd1;
+    (void)fd2;
     return RM_LINK_NONE;
 #endif
 }
@@ -318,7 +320,7 @@ int rm_dedupe_main(int argc, const char **argv) {
                 rm_log_warning_line("dedupe: failed to preserve ownership for %s",
                                     source_path);
                 // try to preserve group ID
-                (void)lchown(cloneto_path, -1, source_stat.st_gid);
+                (void)!lchown(cloneto_path, -1, source_stat.st_gid);
             }
 
             if(lchmod(cloneto_path, source_stat.st_mode) != 0) {
@@ -510,11 +512,10 @@ int rm_is_reflink_main(int argc, const char **argv) {
     g_option_context_free(context);
     g_free(summary);
 
-    if(!HAVE_FIEMAP) {
-        rm_log_error_line(_("Cannot test for reflinks because rmlint was compiled without fiemap support"));
-        return EXIT_FAILURE;
-    }
-
+#if !HAVE_FIEMAP
+    rm_log_error_line(_("Cannot test for reflinks because rmlint was compiled without fiemap support"));
+    return EXIT_FAILURE;
+#else
     const char *a = argv[1];
     const char *b = argv[2];
 
@@ -535,4 +536,5 @@ int rm_is_reflink_main(int argc, const char **argv) {
     rm_log_info("Link type for '%s' and '%s', result:\n", a, b);
     rm_log_warning("%s\n", desc[result]);
     return result;
+#endif
 }