treewide: cleanup, const-correctness, and -Werror

SConstruct

@@ -195,18 +195,30 @@ conf.check_mm_crc32_u64()
 if IS_CLANG := 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:
+    conf.env.Append(CCFLAGS=[
+        '-Wduplicated-cond',
+        '-Wduplicated-branches',
+        '-Wlogical-op',
+    ])
     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',
     '-Wdeprecated-declarations',
 ])
@@ -233,6 +245,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']:
@@ -251,7 +264,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

@@ -33,6 +33,7 @@ CONFIG_VALUES = dict(
     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})
 
@@ -122,6 +123,20 @@ typedef guint64 RmOff;
     #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
 }

lib/shredder.c

@@ -831,7 +831,6 @@ static void rm_shred_group_finalise(RmShredGroup *self) {
         rm_util_thread_pool_push(self->session->shredder->result_pool, self);
         break;
     case RM_SHRED_GROUP_FINISHED:
-    default:
         g_assert_not_reached();
     }
 }
@@ -970,7 +969,6 @@ static RmFile *rm_shred_group_push_file(RmShredGroup *shred_group, RmFile *file,
             g_queue_push_head(shred_group->held_files, file);
             break;
         case RM_SHRED_GROUP_FINISHED:
-        default:
             g_assert_not_reached();
         }
     }

lib/traverse.c

@@ -310,7 +310,6 @@ bool rm_traverse_is_emptydir(const char *path, RmCfg *cfg, int current_depth) {
             rm_log_warning_line(_("error %d in fts_read for %s (skipping)"), errno,
                                 p->fts_path);
             break;
-            break;
         case FTS_SLNONE:  /* symbolic link without target */
         case FTS_W:       /* whiteout object */
         case FTS_NS:      /* rm_sys_stat(2) failed */

lib/utilities.c

@@ -1508,7 +1508,7 @@ bool rm_iso8601_format(time_t stamp, char *buf, gsize buf_size) {
 #define SECONDS_PER_HOUR (60 * 60)
 #define SECONDS_PER_MINUTE (60)
 
-char *rm_format_elapsed_time(gfloat elapsed_sec, int sec_precision) {
+char *rm_format_elapsed_time(gdouble elapsed_sec, int sec_precision) {
     GString *buf = g_string_new(NULL);
 
     if(elapsed_sec > SECONDS_PER_DAY) {

lib/utilities.h

@@ -470,7 +470,7 @@ bool rm_util_thread_pool_push(GThreadPool *pool, gpointer data);
  *
  * @return The formatted string, free with g_free.
  */
-char *rm_format_elapsed_time(gfloat elapsed_sec, int sec_precision);
+char *rm_format_elapsed_time(gdouble elapsed_sec, int sec_precision);
 
 typedef struct {
     gdouble sum;

lib/xattr.c

@@ -238,6 +238,9 @@ int rm_xattr_write_hash(RmFile *file, RmSession *session) {
        rm_xattr_set(file, mtime_key, timestamp, strlen(timestamp), follow)) {
         return errno;
     }
+#else
+    (void)file;
+    (void)session;
 #endif
     return 0;
 }
@@ -286,6 +289,8 @@ gboolean rm_xattr_read_hash(RmFile *file, RmSession *session) {
     file->ext_cksum = g_strdup(cksum_hex_str);
     return TRUE;
 #else
+    (void)file;
+    (void)session;
     return FALSE;
 #endif
 }
@@ -313,13 +318,15 @@ int rm_xattr_clear_hash(RmFile *file, RmSession *session) {
 
     return error;
 #else
+    (void)file;
+    (void)session;
     return EXIT_FAILURE;
 #endif
 }
 
 #if HAVE_XATTR
 
-GHashTable *rm_xattr_list(const char *path, bool follow_symlinks) {
+static GHashTable *rm_xattr_list(const char *path, bool follow_symlinks) {
     const size_t buf_size = 4096;
     const size_t val_size = 1024;
     const char prefix[13] = "user.rmlint.";
@@ -397,9 +404,12 @@ static void rm_xattr_change_subkey(char *key, char *sub_key) {
     strcpy(&key[key_len - sub_key_len], sub_key);
 }
 
+#endif
+
 bool rm_xattr_is_deduplicated(const char *path, bool follow_symlinks) {
     g_assert(path);
 
+#if HAVE_XATTR
     RmStat stat_buf;
     if(rm_sys_stat(path, &stat_buf) < 0) {
         rm_log_warning_line("failed to check dedupe state of %s: %s", path,
@@ -446,11 +456,17 @@ bool rm_xattr_is_deduplicated(const char *path, bool follow_symlinks) {
 
     g_hash_table_destroy(map);
     return result;
+#else
+    (void)path;
+    (void)follow_symlinks;
+    return false;
+#endif
 }
 
 int rm_xattr_mark_deduplicated(const char *path, bool follow_symlinks) {
     g_assert(path);
 
+#if HAVE_XATTR
     RmStat stat_buf;
     if(rm_sys_stat(path, &stat_buf) < 0) {
         rm_log_warning_line("failed to mark dedupe state of %s: %s", path,
@@ -491,6 +507,9 @@ int rm_xattr_mark_deduplicated(const char *path, bool follow_symlinks) {
 
     g_hash_table_destroy(map);
     return result;
-}
-
+#else
+    (void)path;
+    (void)follow_symlinks;
+    return EXIT_FAILURE;
 #endif
+}

site_scons/rm_build_checks.py

@@ -74,6 +74,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
+
+    context.sconf.env['HAVE_STDNORETURN_H'] = rc
+    context.did_show_result = True
+    context.Result(rc)
+    return rc
+
+
 def check_libelf(context):
     rc = 1
 
@@ -387,4 +398,5 @@ def check_cpu_extensions(context):
     'check_cpu_extensions': check_cpu_extensions,
     'check_builtin_cpu_supports': check_builtin_cpu_supports,
     'check_sysmacro_h': check_sysmacro_h,
+    'check_stdnoreturn_h': check_stdnoreturn_h,
 }